Bug 15538 - Use Koha::Logger in longoverdue.pl
Summary: Use Koha::Logger in longoverdue.pl
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Srdjan Jankovic
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-11 03:23 UTC by Srdjan Jankovic
Modified: 2016-08-19 06:14 UTC (History)
5 users (show)

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


Attachments
bug_15538: Koha::Logger in longoverdue.pl (4.29 KB, patch)
2016-01-11 03:37 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug_15538: Koha::Logger in longoverdue.pl (4.29 KB, patch)
2016-05-17 00:33 UTC, Srdjan Jankovic
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Srdjan Jankovic 2016-01-11 03:23:44 UTC
Replace stdout debug prints with proper $logger->debug() calls
Comment 1 Srdjan Jankovic 2016-01-11 03:37:07 UTC Comment hidden (obsolete)
Comment 2 Frédéric Demians 2016-01-15 10:37:59 UTC
I can't make it work on master. Commit 347d12c41f9ebe may have broken Koha::Logger on master. For me, the logger is never instantiated. After modifying Koha::Logger to bypass conf file checking, I get a result in the OPAC log file.
Comment 3 Srdjan Jankovic 2016-01-19 01:50:36 UTC
(In reply to Frédéric Demians from comment #2)
> I can't make it work on master. Commit 347d12c41f9ebe may have broken
> Koha::Logger on master. For me, the logger is never instantiated. After
> modifying Koha::Logger to bypass conf file checking, I get a result in the
> OPAC log file.

Not sure why. I am supplying a conf file via LOG4PERL_CONF variable. What is your situation?
Comment 4 Florent Mara 2016-05-16 22:58:04 UTC
Failed to apply this patch. Appear to be due to Koha::Borrowers now being superseded by Koha::Patrons. 

Applying: bug_15538: Koha::Logger in longoverdue.pl
Using index info to reconstruct a base tree...
M	misc/cronjobs/longoverdue.pl
Falling back to patching base and 3-way merge...
Auto-merging misc/cronjobs/longoverdue.pl
CONFLICT (content): Merge conflict in misc/cronjobs/longoverdue.pl
Failed to merge in the changes.
Patch failed at 0001 bug_15538: Koha::Logger in longoverdue.pl


Could you please provide a test plan to help validate this patch?
Comment 5 Srdjan Jankovic 2016-05-17 00:33:48 UTC
Created attachment 51554 [details] [review]
bug_15538: Koha::Logger in longoverdue.pl

* Removed --verbose option and replaved verbose prints with
  $logger->info()
Comment 6 Florent Mara 2016-05-18 01:12:23 UTC
Applied the following SQL to create a long overdue item and got the expected log print out in stdout.

INSERT INTO biblio(frameworkcode, datecreated) values ('allo', '2016-05-17');
INSERT INTO biblioitems(biblionumber, volume) value ((SELECT biblionumber FROM biblio WHERE frameworkcode = 'allo'), 'blah');
INSERT INTO items(biblionumber, biblioitemnumber, barcode) values (1, (SELECT biblioitemnumber FROM biblioitems WHERE volume = 'blah'), 'barcode');
INSERT INTO items(biblionumber, biblioitemnumber, barcode) values (1, (SELECT biblioitemnumber FROM biblioitems WHERE volume = 'blah'), 'latecode');
INSERT INTO borrowers(surname, address, city, branchcode, categorycode) values ('surname', 'address', 'city', 'CPL', 'B');


INSERT INTO issues (borrowernumber, itemnumber, date_due) values (
(SELECT borrowernumber FROM borrowers WHERE surname = 'surname'), 
(SELECT itemnumber FROM items WHERE barcode = 'barcode'), '2016-05-17'); 

INSERT INTO issues (borrowernumber, itemnumber, date_due) values (
(SELECT borrowernumber FROM borrowers WHERE surname = 'surname'), 
(SELECT itemnumber FROM items WHERE barcode = 'latecode'), '2015-07-17');
Comment 7 Marcel de Rooy 2016-08-19 06:14:44 UTC
Do we really want to replace verbose options in cmdline scripts by logger calls?
The first comment speaks about debug prints, but you are removing the verbose option too.
I would like to see some more consensus about doing so. Maybe ask the dev list?

In Discussion