Bug 8826

Summary:	Initialization of hash triggers error log entries.
Product:	Koha	Reporter:	Mark Tompsett <mtompset>
Component:	Circulation	Assignee:	Mark Tompsett <mtompset>
Status:	CLOSED FIXED	QA Contact:	Paul Poulain <paul.poulain>
Severity:	minor
Priority:	P5 - low	CC:	chris, gmcharlt, kyle.m.hall, kyle, liz, paul.poulain
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Changed {} to () in initialization of variable. Changed {} to () in initialization of variable. (3.8.x) [SIGNED-OFF] Bug 8826 - Initialization of hash triggers error log entries. Bug 8826 - Initialization of hash triggers error log entries - Followup Bug 8826 - Initialization of hash triggers error log entries - Followup

Description Mark Tompsett 2012-09-26 08:40:43 UTC

Bug 7851 initialized a hash incorrectly.
our %renew_failed = {};
This attempts to create a hash of a single anonymous hash reference which generates the error:
circulation.pl: Reference found where even-sized list expected at /usr/share/koha/intranet/cgi-bin/circ/circulation.pl line 97.

As I do not know how my proposed change of:
our %renew_failed = ();
will work in a plack context, I would need someone to assist in testing that.

I was thinking of reverting to no initialization, but based on my reading of bug 7851, that isn't an option.

In the worst case scenario:
our %renew_failed = ( "dummyplaceholderkey" => "dummyplaceholdervalue" );
should work.

This only applies to 3.8.x and master, as 3.6.x still has an uninitialized value (which should not be problematic as far as I can tell).

Comment 1 Mark Tompsett 2012-09-27 07:59:54 UTC Comment hidden (obsolete)

Created attachment 12543 [details] [review]
Changed {} to () in initialization of variable.

This is the master version of the patch. 3.8.x needs a different patch.

Test
-----
1) Login in to staff client
2) Click 'Circulation' (doesn't matter which one)
3) Click the 'Check Out' link below the Circulation title, and above the 'Check In' link.
4) Check the intranet error log. There should be an error like:
[Thu Sep 27 14:51:06 2012] [error] [client 192.168.100.2] [Thu Sep 27 14:51:06 2012] circulation.pl: Reference found where even-sized list expected at /usr/share/koha/intranet/cgi-bin/circ/circulation.pl line 97.
5) Apply patch
6) repeat steps 1-3, and recheck for an error. There shouldn't be one.

FURTHER TEST
-------------
Do this on a plakified setup. I have not tested this case, but it is likely important as bug 7851 changed this single line in the first place.

Comment 2 Mark Tompsett 2012-09-27 08:01:39 UTC

Created attachment 12544 [details] [review]
Changed {} to () in initialization of variable. (3.8.x)

Changes between 3.8.x and master require a separate patch to apply cleanly. The test plan should be the same as master.

Comment 3 Kyle M Hall (khall) 2012-09-28 13:18:57 UTC

Created attachment 12581 [details] [review]
[SIGNED-OFF] Bug 8826 - Initialization of hash triggers error log entries.

Changed:
our %renew_failed = {};
To:
our %renew_failed = ();
This prevents an error log about Reference found where
even-sized list expected.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Comment 4 Kyle M Hall (khall) 2012-09-28 13:20:23 UTC Comment hidden (obsolete)

Created attachment 12582 [details] [review]
Bug 8826 - Initialization of hash triggers error log entries - Followup

Comment 5 Mark Tompsett 2012-10-05 11:41:56 UTC

Thanks for catching the follow up, Kyle. I was just about to add that.

Comment 6 Kyle M Hall (khall) 2012-10-05 12:10:50 UTC

(In reply to comment #5)
> Thanks for catching the follow up, Kyle. I was just about to add that.

No problem! Can you give it a test and a sign-off?

Comment 7 Chris Cormack 2012-11-01 06:59:20 UTC

Created attachment 13165 [details] [review]
Bug 8826 - Initialization of hash triggers error log entries - Followup

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Comment 8 Paul Poulain 2012-11-02 11:20:18 UTC

QA comment: tiny (2 lines) patch, passes QA

Comment 9 Jared Camins-Esakov 2012-11-02 16:13:12 UTC

This patch has been pushed to master.

Comment 10 Paul Poulain 2012-11-02 16:46:00 UTC

Patch pushed to branch 3.10.x

Comment 11 Chris Cormack 2012-11-02 17:19:17 UTC

3.8.x patch pushed, will be in 3.8.7

Comment 12 Liz Rea 2012-11-28 01:47:22 UTC

Patches do not apply to 3.6.x, please reopen and submit a patch for 3.6.x if necessary. 

Thanks!

Comment 13 Mark Tompsett 2012-11-28 18:44:59 UTC

Bug 8880 includes the changes here in 8826. So if 8880 has been been applied to 3.6.x then that explains why this can't be applied. I haven't looked further yet.