Bug 15795

Summary: C4/Members.pm is floody (Norwegian Patron DB)
Product: Koha Reporter: Mark Tompsett <mtompset>
Component: PatronsAssignee: Mark Tompsett <mtompset>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: trivial    
Priority: P5 - low CC: brendan, f.demians, gmcharlt, jonathan.druart, julian.maurice, kyle.m.hall, m.de.rooy, magnus, philippe.blouin
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8483
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 15151    
Bug Blocks:    
Attachments: Bug 15795: C4/Members.pm is floody (Norwegian Patron DB)
[SIGNED-OFF] Bug 15795: C4/Members.pm is floody (Norwegian Patron DB)
Bug 15795: C4/Members.pm is floody (Norwegian Patron DB)

Description Mark Tompsett 2016-02-11 02:27:52 UTC
A warn without a prepended $debug is causing floodiness when NorwegianPatronDB is not set.
Comment 1 Mark Tompsett 2016-02-11 02:51:56 UTC
Though I can't see how to trigger it, except by deleting ~/kohaclone/Koha/NorwegianPatronDB.pm, there probably should be a $debug prepended on the warn.
Comment 2 Mark Tompsett 2016-02-11 03:03:41 UTC Comment hidden (obsolete)
Comment 3 Mark Tompsett 2016-02-11 03:12:06 UTC
See the conversation on Nabble: http://koha.1045719.n5.nabble.com/Introduce-the-use-of-Grunt-or-Gulp-tp5873339p5873842.html

Sadly, it got inter-threaded. The dangers of just replying in order to create a new thread. :(
Comment 4 Magnus Enger 2016-02-11 08:59:55 UTC
How about getting rid of both the if and the warn? So from this:

 use Module::Load::Conditional qw( can_load );
 if ( ! can_load( modules => { 'Koha::NorwegianPatronDB' => undef } ) ) {
    warn "Unable to load Koha::NorwegianPatronDB";
 }

to this:

 use Module::Load::Conditional qw( can_load );
 can_load( modules => { 'Koha::NorwegianPatronDB' => undef } );
Comment 5 Mark Tompsett 2016-02-11 13:11:13 UTC
(In reply to Magnus Enger from comment #4)
> How about getting rid of both the if and the warn? So ... this:
> 
>  use Module::Load::Conditional qw( can_load );
>  can_load( modules => { 'Koha::NorwegianPatronDB' => undef } );

While functional, I think warns are a good thing. I would rather keep the potential to warn, hence my patch.
Comment 6 Blou 2016-02-11 14:59:17 UTC Comment hidden (obsolete)
Comment 7 Blou 2016-02-11 15:04:17 UTC
Although I would prefer Magnus' code snippet overall, because it's less ugly than the big "if" that gives too much importance to that feature IMO, adding $debug is following the standard and expectations.  

I like standards...
Comment 8 Mark Tompsett 2016-02-11 18:26:23 UTC
(In reply to Blou from comment #7)
> Although I would prefer Magnus' code snippet overall,
> because it's less ugly

-- totally agree.


> than the big "if" that gives too much importance to that feature IMO, adding
> $debug is following the standard and expectations.  
> 
> I like standards...

And the discussion on development mailing list may end up turning this functionality into a plug-in in the future, but until then, this suffices. +1 to future plugin. :)
Comment 9 Marcel de Rooy 2016-02-12 07:14:49 UTC
Created attachment 47948 [details] [review]
Bug 15795: C4/Members.pm is floody (Norwegian Patron DB)

From the mailing list:
"I'm working on 3.22.02 and my logs are FILLED with the following warnings:

Members.pm: Unable to load Koha::NorwegianPatronDB at /blurred/C4/Members.pm line 46., referer: http://blurred/cgi-bin/koha/acqui/booksellers.pl
admin-home.pl: Unable to load Koha::NorwegianPatronDB at /blurred/C4/Members.pm line 46., referer: http://blurred/cgi-bin/koha/admin/preferences.pl"

I agree. Looking at C4/Members.pm other warns are all prepended with $debug.

TEST PLAN
---------
1) rm Koha/NorwegianPatronDB.pm
   -- quickest and dirtiest way to get the load to fail.
2) echo > ~/koha-dev/var/log/koha-error_log
   -- so it will be easy to spot the log changes.
   -- back up the log file if you really want it.
3) log into the staff client and go to patrons.
   -- Check the log file. "Unable to load ..." will be there.
4) echo > ~/koha-dev/var/log/koha-error_log
5) sudo vi /etc/apache2/sites-enabled/{your koha site file}
   -- Find your staff client entry in your apache
      configuration file and add:
       SetEnv DEBUG 1
6) sudo service apache2 restart
7) apply the patch
8) refresh the patron page
   -- "Unable to load..." will still be there.
9) change the DEBUG value in your apache config to 0
10) sudo sevice apache2 restart
11) echo > ~/koha-dev/var/log/koha-error_log
12) refresh the patron page
    -- "Unable to load..." will NOT be there.
13) git checkout origin/master -- Koha/NorwegianPatronDB.pm
    -- to bring it back.
14) refresh the patron page
    -- "Unable to load..." will still not be there.
15) run koha qa test tools

Signed-off-by: Philippe Blouin <philippe.blouin@inlibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Brendan Gallagher 2016-02-24 03:10:14 UTC
Pushed to Master - Should be in the May 2016 release.  (Just a note one of the previous patches wasn't obsolete - I followed up on that).  Thanks!
Comment 11 Julian Maurice 2016-02-26 15:43:22 UTC
Patch pushed to 3.22.x, will be in 3.22.4
Comment 12 Frédéric Demians 2016-03-01 16:54:57 UTC
This patch has been pushed to 3.20.x, will be in 3.20.9.