Bug 10494

Summary: KohaBranchName sends useless warnings to log if supplied library code is null or not present
Product: Koha Reporter: Galen Charlton <gmcharlt>
Component: TemplatesAssignee: Galen Charlton <gmcharlt>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: trivial    
Priority: P5 - low CC: bgkriegel, katrin.fischer, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 10494: test cases for the KohaBranchName plugin
Bug 10494: remove spurious warnings from the KohaBranchName plugin
Bug 10494: test cases for the KohaBranchName plugin
Bug 10494: remove spurious warnings from the KohaBranchName plugin
[PASSED QA] Bug 10494: test cases for the KohaBranchName plugin
[PASSED QA] Bug 10494: remove spurious warnings from the KohaBranchName plugin

Description Galen Charlton 2013-06-19 15:35:21 UTC
If the library code passed to the KohaBranchName Template Toolkit plugin is NULL or isn't present in the database (which can happen, legitimately, in suggestion management), spurious warnings can get logged.

For example, here's one that gets displayed when viewing purchase suggestions in the staff interface if any of the suggestions have branchcode either NULL or set to '__ANY__'.

[Wed Jun 19 07:13:06 2013] [error] [client 192.168.1.10] [Wed Jun 19 07:13:06 2013] suggestion.pl: Use of uninitialized value in concatenation (.) or string at /home/gmc/koha/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt line 487., referer: http://koha-staff.metaquarry/cgi-bin/koha/acqui/acqui-home.pl

Rather than return undef in such cases, the plugin should return the empty string.
Comment 1 Galen Charlton 2013-06-19 15:58:35 UTC Comment hidden (obsolete)
Comment 2 Galen Charlton 2013-06-19 15:58:38 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2013-06-30 08:25:09 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2013-06-30 08:25:26 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2013-06-30 17:17:42 UTC
Created attachment 19301 [details] [review]
[PASSED QA] Bug 10494: test cases for the KohaBranchName plugin

To test, in a test database that has the sample branches
loaded, run

prove -v t/db_dependent/Koha_template_plugin_KohaBranchName.t

Note that this includes regression tests for the issues
reported in bug 10494; these won't pass without applying
the patch that fixes them.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 6 Katrin Fischer 2013-06-30 17:17:56 UTC
Created attachment 19302 [details] [review]
[PASSED QA] Bug 10494: remove spurious warnings from the KohaBranchName plugin

The KohaBranchName plugin now returns an empty string if
given a NULL (undefined) library code or a library code that
is not present in the database (e.g., the '__ANY__' placeholder
that purchase suggestions can use).

To test:

[1] Create a purchase suggestion, setting the library
    to 'Any'.  Create a second one setting the library to
    a specific one.
[2] Go to the suggestions overview page in the staff interface.
    When you do so, the Apache logs should contain a warning
    that looks like this:

[Wed Jun 19 07:13:06 2013] [error] [client 192.168.1.10] [Wed Jun 19 07:13:06 2013] suggestion.pl: Use of uninitialized value in concatenation (.) or string at /home/koha/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt line 487., referer: http://koha-staff.metaquarry/cgi-bin/koha/acqui/acqui-home.pl

[3] Apply the patch.
[4] Refresh the suggestions overview page.  This time, there should be
    no new warning logged.
[5] Verify that the library names for the two pending suggestions
    created in step 1 are displayed correctly.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script. Also adds new tests!

Note: For the test plan to work you have to add the suggestion
from the intranet, as Any is not an available option in OPAC.
Comment 7 Galen Charlton 2013-07-01 01:45:53 UTC
Pushed to master.
Comment 8 Tomás Cohen Arazi 2013-07-15 18:10:43 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Galen!
Comment 9 Bernardo Gonzalez Kriegel 2013-08-02 20:18:54 UTC
Pushed to 3.10.x, will be in 3.10.10