Bug 7937

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.

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>

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>

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>

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>

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>

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.

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

Katrin, any feedback about my comment 14

filing QA because of comment 14, hoping that will motivate katrin to answer ;-)

I want to continue working on this - but I have to find time :(

*** Bug 9040 has been marked as a duplicate of this bug. ***

Is this patch set still valid?

(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.

Very old patch set - best to start over on new bugs.