Bug 10126

Summary: C4::Reports::Guided is not Plack safe
Product: Koha Reporter: Jared Camins-Esakov <jcamins>
Component: Architecture, internals, and plumbingAssignee: Galen Charlton <gmcharlt>
Status: CLOSED FIXED QA Contact:
Severity: major    
Priority: P5 - low CC: jonathan.druart, paul.poulain, tomascohen
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:
Bug Depends on:    
Bug Blocks: 7172    
Attachments: Bug 10126: Remove my variables at module level
Bug 10126: Remove my variables at module level
Bug 10126: Remove my variables at module level
Bug 10126: (qa followup) fix tests

Description Jared Camins-Esakov 2013-04-26 12:06:24 UTC
Notice then numerous my variables at module level. There is also a call to C4::Context->preference in a BEGIN block, which is bad.
Comment 1 Jonathan Druart 2014-10-09 15:20:28 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-10-09 15:20:59 UTC
This patch is quite ugly, feel free to provide your own version ;)
Comment 3 Chris Cormack 2014-10-09 15:52:22 UTC Comment hidden (obsolete)
Comment 4 Paul Poulain 2014-10-10 19:44:16 UTC
QA comments :
 * passes koha-qa.pl
 * guided reports also still work in CGI mode
 * no other comments, looks good

passed QA
Comment 5 Paul Poulain 2014-10-10 19:44:51 UTC
Created attachment 32199 [details] [review]
Bug 10126: Remove my variables at module level

In C4::Reports::Guided, a lot of variables was defined at module level
and reused in subroutine.

I didn't find any problem with Plack, so I'm not sure this patch is
useful.

Test plan:
Verify there is no more ^my in the module and you don't find any
regression with the guided reports tools (with and without Plack)

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

I'm unsure if this is needed, but I have verified it causes no
regressions

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 6 Tomás Cohen Arazi 2014-10-15 21:27:54 UTC
Patch pushed to master.

Thanks Jonathan.
Comment 7 Tomás Cohen Arazi 2014-10-16 13:26:28 UTC
Created attachment 32409 [details] [review]
Bug 10126: (qa followup) fix tests

It seems the removal of global variables changes the behaviour
of Test::MockModule, and it now expects the namespace in front
of the statically called method.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>