Bug 26137 - Warn on malformed param on log viewer (viewlog.pl)
Summary: Warn on malformed param on log viewer (viewlog.pl)
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-04 19:24 UTC by Nick Clemens
Modified: 2020-11-17 20:32 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.06


Attachments
Bug 26137: Prevent warns form malformed params (1.64 KB, patch)
2020-08-04 19:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26137: Prevent warns form malformed params (1.53 KB, patch)
2020-09-29 18:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26137: Prevent warns form malformed params (1.59 KB, patch)
2020-09-30 11:48 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 26137: Prevent warns form malformed params (1.65 KB, patch)
2020-10-02 08:38 UTC, Julian Maurice
Details | Diff | Splinter Review
[ALTERNATIVE PATCH] Bug 26137: Remove warnings from viewlog.pl (1.00 KB, patch)
2020-10-08 09:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26137: Remove warnings from viewlog.pl (1.06 KB, patch)
2020-10-09 10:30 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-08-04 19:24:34 UTC
In bug 25893 I noticed a warn when loading log viewer.

After some investigation I believe it comes from:
 94     subscriptionsnumber => CountSubscriptionFromBiblionumber(scalar $input->param('object')),

This may not return a value, if we don't have an object - when it doesn't we get a warn
Comment 1 Nick Clemens 2020-08-04 19:38:29 UTC
Created attachment 107804 [details] [review]
Bug 26137: Prevent warns form malformed params

To test:
1 - Tail plack logs
2 - Browse to Tools->Log viewer
3 - Note an error
4 - Apply patch
5 - Confirm error is gone
6 - Confirm everytihg stil works right
Comment 2 Fridolin Somers 2020-09-03 12:55:33 UTC
> my $subscriptionsnumber = CountSubscriptionFromBiblionumber(scalar $input->param('object'));
Why not CountSubscriptionFromBiblionumber($object) ?

And if object is empty maybe not call this method.
Comment 3 Fridolin Somers 2020-09-29 10:08:23 UTC
Needs a rebase since Bug 25893 is integrated
Comment 4 Nick Clemens 2020-09-29 18:14:06 UTC
Created attachment 110936 [details] [review]
Bug 26137: Prevent warns form malformed params

To test:
1 - Tail plack logs
2 - Browse to Tools->Log viewer
3 - Note an error
4 - Apply patch
5 - Confirm error is gone
6 - Confirm everytihg stil works right
Comment 5 Fridolin Somers 2020-09-30 11:48:59 UTC
Created attachment 110981 [details] [review]
Bug 26137: Prevent warns form malformed params

To test:
1 - Tail plack logs
2 - Browse to Tools->Log viewer
3 - Note an error
4 - Apply patch
5 - Confirm error is gone
6 - Confirm everytihg stil works right

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 6 Fridolin Somers 2020-09-30 11:50:02 UTC
Ok does the job.

I understand its for the number in left tab of biblio pages : Subscriptions (2)
Comment 7 Fridolin Somers 2020-09-30 11:53:23 UTC
I think we should after this bug review the template 
koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc :
    [% IF ( subscriptionsnumber ) %]<li><a href="/cgi-bin/koha/serials/serials-search.pl?searched=1&amp;biblionumber=[% biblio_object_id | url  %]">Subscriptions ([% subscriptionsnumber | html %])</a></li>[% END %]

I propose we create a new method in Koha/Template/Plugin/Biblio.pm : 
SubscriptionsCount, like HoldsCount
Comment 8 Julian Maurice 2020-10-02 08:38:30 UTC
Created attachment 111073 [details] [review]
Bug 26137: Prevent warns form malformed params

To test:
1 - Tail plack logs
2 - Browse to Tools->Log viewer
3 - Note an error
4 - Apply patch
5 - Confirm error is gone
6 - Confirm everytihg stil works right

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 9 Jonathan Druart 2020-10-08 09:28:22 UTC
(In reply to Fridolin SOMERS from comment #7)
> I think we should after this bug review the template 
> koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc :
>     [% IF ( subscriptionsnumber ) %]<li><a
> href="/cgi-bin/koha/serials/serials-search.pl?searched=1&amp;biblionumber=[%
> biblio_object_id | url  %]">Subscriptions ([% subscriptionsnumber | html
> %])</a></li>[% END %]
> 
> I propose we create a new method in Koha/Template/Plugin/Biblio.pm : 
> SubscriptionsCount, like HoldsCount

No, please don't create TT plugin method for that. The Koha::Biblio object's methods must be called instead.
Comment 10 Jonathan Druart 2020-10-08 09:31:39 UTC
The test is weird, I would have expected to call CountSubscriptionFromBiblionumber only if the "object" param is passed.
Comment 11 Jonathan Druart 2020-10-08 09:34:27 UTC
Created attachment 111367 [details] [review]
[ALTERNATIVE PATCH] Bug 26137: Remove warnings from viewlog.pl

The warnings are
Use of uninitialized value $val in concatenation (.) or string at /kohadevbox/koha/C4/Templates.pm line 138.
Problem = a value of  has been passed to param without key at /kohadevbox/koha/C4/Templates.pm line 138.
Comment 12 Jonathan Druart 2020-10-08 09:35:04 UTC
Julian, Nick, Frido, what about this alternative patch?
Comment 13 Nick Clemens 2020-10-09 10:30:11 UTC
Created attachment 111390 [details] [review]
Bug 26137: Remove warnings from viewlog.pl

The warnings are
Use of uninitialized value $val in concatenation (.) or string at /kohadevbox/koha/C4/Templates.pm line 138.
Problem = a value of  has been passed to param without key at /kohadevbox/koha/C4/Templates.pm line 138.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Nick Clemens 2020-10-09 10:30:44 UTC
(In reply to Jonathan Druart from comment #12)
> Julian, Nick, Frido, what about this alternative patch?

That works and is simple, good for me
Comment 15 Jonathan Druart 2020-10-09 11:02:31 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 16 Lucas Gass 2020-11-10 22:29:01 UTC
backported to 20.05.x for 20.05.06
Comment 17 Aleisha Amohia 2020-11-17 03:35:42 UTC
enhancement, not backported to 19.11.x