Bug 14186 - Noisy warns in opac-reserve.pl
Summary: Noisy warns in opac-reserve.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-12 05:40 UTC by Aleisha Amohia
Modified: 2016-12-05 21:23 UTC (History)
5 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:


Attachments
Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl (1.17 KB, patch)
2015-05-12 05:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl (1.59 KB, patch)
2015-05-14 01:35 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
[FOLLOWUP] Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl (1.72 KB, patch)
2015-05-14 02:09 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
[PASSED QA] Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl (1.66 KB, patch)
2015-05-22 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl (1.78 KB, patch)
2015-05-22 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl (1.66 KB, patch)
2015-05-22 12:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 14186 [QA Followup]: Undefined $reservedfor causes warn in opac-reserve.pl (1.80 KB, patch)
2015-05-22 12:31 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-05-12 05:40:13 UTC
If $reservedfor is undefined, a warn comes up in error log
Comment 1 Aleisha Amohia 2015-05-12 05:48:23 UTC Comment hidden (obsolete)
Comment 2 Indranil Das Gupta 2015-05-13 09:21:10 UTC
Comment on attachment 39057 [details] [review]
Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl

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

Hi Aleisha,

Would you be considering making the small change mtompset@hotmail.com suggested in bug 14185, comment 3 for this patch?

If you do, then I'll wait for your update, before I sign off on this.
Comment 3 Indranil Das Gupta 2015-05-14 01:08:10 UTC
(In reply to Indranil Das Gupta from comment #2)
> Comment on attachment 39057 [details] [review] [review]
> Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl
> 
> Review of attachment 39057 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> Hi Aleisha,
> 
> Would you be considering making the small change mtompset@hotmail.com
> suggested in bug 14185, comment 3 for this patch?
> 
> If you do, then I'll wait for your update, before I sign off on this.

^^^ the question is no longer relevant. Will proceed with testing.
Comment 4 Indranil Das Gupta 2015-05-14 01:35:23 UTC Comment hidden (obsolete)
Comment 5 Indranil Das Gupta 2015-05-14 01:41:31 UTC
Comment on attachment 39175 [details] [review]
Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl

Result of QA test

 FAIL	opac/opac-reserve.pl
   OK	  critic
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 470)
   OK	  pod
   OK	  valid
Comment 6 Indranil Das Gupta 2015-05-14 02:09:10 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2015-05-22 12:30:08 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2015-05-22 12:30:14 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2015-05-22 12:31:27 UTC
Created attachment 39401 [details] [review]
[PASSED QA] Bug 14186: Undefined $reservedfor causes warn in opac-reserve.pl

This patch sets $reservedfor to an empty string.

Test plan
=========

1/ in a terminal, run `tail -f ` on your instance's opac-error.log
2/ go to the opac and search from an item that exists on the Koha
   instance.
3/ Select the title (if more than one title is returned) and click on
   'Place hold' link to go to opac-reserve.pl
4/ notice the warning - "opac-reserve.pl: Use of uninitialized value
   $reservedfor" appear in the `tail`ed opac-error.log
5/ apply the patch
6/ reload the page (opac-reserve.pl)
7/ page works but the warning in step #4 is no longer thrown up
8/ run qa test (i.e. koha-qa.pl -c 1 -v 2), there should be no error

Remarks: The QA test failed - "forbidden pattern: tab char (line 470)".

         Marking this as 'FAILED QA'

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2015-05-22 12:31:34 UTC
Created attachment 39402 [details] [review]
[PASSED QA] Bug 14186 [QA Followup]: Undefined $reservedfor causes warn in opac-reserve.pl

This is a followup for Bug 14186 that removes the extraneous tab
char on line 470, so that the patch can clear QA tools.

This patch sets $reservedfor to an empty string.

Test plan
=========

1/ in a terminal, run `tail -f ` on your instance's opac-error.log
2/ go to the opac and search from an item that exists on the Koha
   instance.
3/ Select the title (if more than one title is returned) and click on
   'Place hold' link to go to opac-reserve.pl
4/ notice the warning - "opac-reserve.pl: Use of uninitialized value
   $reservedfor" appear in the `tail`ed opac-error.log
5/ apply the patch
6/ reload the page (opac-reserve.pl)
7/ page works but the warning in step #4 is no longer thrown up
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 11 Tomás Cohen Arazi 2015-05-26 14:05:45 UTC
Patch pushed to master.

Thanks Aleisha and congratulations Indranil for your first patch!
Comment 12 Chris Cormack 2015-05-27 04:26:44 UTC
Pushed to 3.20.x will be in 3.20.1
Comment 13 Liz Rea 2015-06-22 22:35:34 UTC
Patch pushed to 3.18.x will be in 3.18.08
Comment 14 Mason James 2015-07-25 09:33:33 UTC
Pushed to 3.16.x, will be in 3.16.13