Bug 7937 - Silence warns in OPAC error log (koha-opac-error_log)
Summary: Silence warns in OPAC error log (koha-opac-error_log)
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
: 9040 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-04-09 20:16 UTC by Katrin Fischer
Modified: 2018-06-04 20:10 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7937: Silence warns in OPAC error log (koha-opac-error_log) (2.03 KB, patch)
2012-04-09 21:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7937: Silence warns in OPAC error log (koha-opac-error_log) (2.25 KB, patch)
2012-04-09 22:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7937: Fix warns on opac-userupdate.pl (2.90 KB, patch)
2012-04-09 23:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7937: Fix warns in opac-authoritiesdetail.pl (1.17 KB, patch)
2012-04-10 00:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7937: Fixes warns in authority search (C4/Authorities.pm) (4.77 KB, patch)
2012-04-10 00:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7937: Fix warns in opac-reserve.pl (1.57 KB, patch)
2012-04-10 06:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7937: Silence warns in OPAC error log (koha-opac-error_log) (2.18 KB, patch)
2012-04-11 13:05 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED OFF] Bug 7937: Silence warns in OPAC error log (koha-opac-error_log) (2.32 KB, patch)
2012-04-12 09:14 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED OFF] Bug 7937: Fix warns on opac-userupdate.pl (2.96 KB, patch)
2012-04-12 09:18 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED OFF] Bug 7937: Fix warns in opac-authoritiesdetail.pl (1.24 KB, patch)
2012-04-12 09:41 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED OFF] Bug 7937: Fixes warns in authority search (C4/Authorities.pm) (4.84 KB, patch)
2012-04-12 09:45 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED OFF] Bug 7937: Fix warns in opac-reserve.pl (1.64 KB, patch)
2012-04-12 09:50 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2012-04-09 20:16:48 UTC

    
Comment 1 Katrin Fischer 2012-04-09 21:14:30 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-04-09 22:42:26 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2012-04-09 23:20:39 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2012-04-10 00:11:33 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2012-04-10 00:11:47 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2012-04-10 06:42:30 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2012-04-11 13:05:13 UTC
Created attachment 9115 [details] [review]
[SIGNED-OFF] Bug 7937: Silence warns in OPAC error log (koha-opac-error_log)

To reproduce:
Access a detail page where at least some items are available and have
no due date.

Log:
opac-detail.pl: Use of uninitialized value in concatenation (.) or string at
/home/katrin/kohaclone/koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
line 657.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Entry in the log is gone after the patch is applied.
Comment 8 Chris Cormack 2012-04-12 09:14:38 UTC
Created attachment 9134 [details] [review]
[SIGNED OFF] Bug 7937: Silence warns in OPAC error log (koha-opac-error_log)

I noticed warnings in the logs for an item without itemtype on item
and record level. While trying to resolve the warns, I noticed a possible
optimization in XSLT.pm.

GetItemsInfo is built to return the notforloan value both for the item
and record level, depending on the item-level_itype setting.

- $item->{notforloan} doesn't exist in @items
- $itemtypes->{ $item->{itype} }->{notforloan} can be replaced by $item->{notforloan_per_itemtype}

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 9 Chris Cormack 2012-04-12 09:18:14 UTC
Created attachment 9135 [details] [review]
[SIGNED OFF] Bug 7937: Fix warns on opac-userupdate.pl

1) opac-userupdate.pl: Use of uninitialized value $updateemailaddress in
pattern match (m//) at /home/katrin/kohaclone/opac/opac-userupdate.pl line 61.

Warn was created when branch has no email address.
The script falls back to KohaAdminEmailAddress first and shows an
error message in OPAC when there is no email address to be used.

2) opac-userupdate.pl: Use of uninitialized value in concatenation (.) or
string at /home/katrin/kohaclone/koha-tmpl/opac-tmpl/prog/en/modules/opac-userupdate.tt
line 75.

Warn was created when patron has no date of birth.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 10 Chris Cormack 2012-04-12 09:41:00 UTC
Created attachment 9141 [details] [review]
[SIGNED OFF] Bug 7937: Fix warns in opac-authoritiesdetail.pl

[Tue Apr 10 01:51:20 2012] opac-authoritiesdetail.pl: Use of
uninitialized value in concatenation (.) or string line 162

Warn is created, when authority record contains a field that does
not exist in the authorities frameworks.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 11 Chris Cormack 2012-04-12 09:45:46 UTC
Created attachment 9142 [details] [review]
[SIGNED OFF] Bug 7937: Fixes warns in authority search (C4/Authorities.pm)

Errors were output when searching for authorities in OPAC.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 12 Chris Cormack 2012-04-12 09:50:31 UTC
Created attachment 9143 [details] [review]
[SIGNED OFF] Bug 7937: Fix warns in opac-reserve.pl

1) opac-reserve.pl: Use of uninitialized value in string ne at
/home/katrin/kohaclone/opac/opac-reserve.pl line 475.

2) opac-reserve.pl: Use of uninitialized value $reservedfor in
string eq at /home/katrin/kohaclone/opac/opac-reserve.pl line 410

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 13 Ian Walls 2012-04-16 18:14:48 UTC
First patch:  fixes some spacing, then adds conditional check to template var before printing.

second patch: C4 change.  Replaces usage of $itemtypes with $item->{notforloan_per_itemtype}, making the subroutine buildKohaItemsNamespace more efficient

third patch: fixes spacing and conditional check on template, and provides default value for possibly undefined variable.

fourth patch: conditional check for variable before concatenating

fifth patch: C4 change.  Introduces conditional checks into SearchAuthorities().  Lots of text moved to fix indenting, but functional changes are minimal.

sixth patch: better conditional checks on variables.


Marking all 6 Passed QA.
Comment 14 Paul Poulain 2012-04-19 08:39:35 UTC
About patch 1, shouldn't we fix the KohaDate itself, to return '' if there is no date provided ?

Something like:
     my ($self,$text) = @_;
+    return '' unless $text;
    my $date = C4::Dates->new( $text, 'iso' );
    return $date->output("syspref");


patch 3 would also have to be rewriten in this case.
(This option would result in silencing all KohaDates warning isn't it ?)

I agree with patch 2 and 4

I think patch 5 is OK, but there maybe some side effect replacing $x ne 1 by !$x (even if I think there is none in this case). I'll delay this one for after 3.8.0 release, just to be carefull.

Waiting for feedback on counter-patch for 1 and 3
Comment 15 Paul Poulain 2012-05-07 20:10:34 UTC
Katrin, any feedback about my comment 14
Comment 16 Paul Poulain 2012-05-11 09:02:27 UTC
filing QA because of comment 14, hoping that will motivate katrin to answer ;-)
Comment 17 Katrin Fischer 2012-05-11 09:06:38 UTC
I want to continue working on this - but I have to find time :(
Comment 18 Tomás Cohen Arazi 2013-08-27 17:06:42 UTC
*** Bug 9040 has been marked as a duplicate of this bug. ***
Comment 19 Marc Véron 2016-10-04 16:10:32 UTC
Is this patch set still valid?
Comment 20 Mark Tompsett 2016-11-21 17:16:20 UTC
(In reply to Marc Véron from comment #19)
> Is this patch set still valid?

As a general concept, yes, I believe so. These patches specifically? I don't know.
Comment 21 Katrin Fischer 2017-08-19 08:16:06 UTC
Very old patch set - best to start over on new bugs.