Bug 5878

Summary: overdue printing fixes and improvements
Product: Koha Reporter: Chris Cormack <chris>
Component: CirculationAssignee: Paul Poulain <paul.poulain>
Status: CLOSED DUPLICATE QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: P1 - high CC: gmcharlt, katrin.fischer, koha.sekjal, sophie.meynieux
Version: 3.6   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 5575    
Attachments: Proposed Patch
rebased patch
follow-up fixing perl warnings
only removing some warn left
screenshot for 7)

Description Chris Cormack 2011-03-16 02:42:09 UTC
* misc/cronjob/printoverdues deals with utf-9 correctly
* misc/cronjob/overdue_notices
- display the due_date
- deals with a branchcode parameter limit
- deals with the various emails a patron has to select the 1st valid one (function in C4/Members)
Comment 1 Chris Cormack 2011-03-16 02:51:41 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2011-03-19 17:27:15 UTC
Tried testing this, here are my notes:

1) The default settings for items.content do not make much sense to me:

-my $itemscontent = join( ',', qw( date_due title barcode author itemnumber ) );
+my $itemscontent = join( ',', qw( issuedate title barcode author biblionumber date_due) );

itemnumber is better than biblionumber, although both are only confusing for patrons. date_due makes sense. I would perhaps suggest:

my $itemscontent = join( ',', qw( date_due title author callnumber barcode) )

2) Running overdue_notices.pl creates a lot of output on command line:

no query in themelanguage at /home/katrin/kohaclone/C4/Output.pm line 152.
---
altcity: Alternative City
altnotes: Alternative Contact Notes
altphone: Alternative Phone
altrelationship: Relationship of Alternative Contact
altstreetaddress: ' Alternative Street Address'
altsuburb: Alternative Suburb
area: Area
author: Author
barcode: Barcode
biblio.author: Author
biblio.datecreated: Creation Date
biblio.frameworkcode: Framework Code
biblio.timestamp: Modification Date
[...]

3) Using -html to create print notices with <item> syntax in overdue template creates a lot of error messages (additional to output above)

ODUE:
<item>"<<biblio.title>>" by <<biblio.author>>, <<items.itemcallnumber>>, Barcode: <<items.barcode>> Fine: <fine>EUR</fine></item>

Error messages from overdue_notices.pl with patch applied:
Use of uninitialized value in join or string at ./overdue_notices.pl line 502.
Use of uninitialized value in join or string at ./overdue_notices.pl line 502.
issuedatetitlebarcodeauthorbiblionumberdate_due at ./overdue_notices.pl line 503.
Use of uninitialized value in concatenation (.) or string at ./overdue_notices.pl line 504.
Use of uninitialized value in concatenation (.) or string at ./overdue_notices.pl line 504.
;Title;Barcode;Author;Biblio Number (koha internal);; at ./overdue_notices.pl line 504.
Use of uninitialized value $fine in string at ./overdue_notices.pl line 706.
Use of uninitialized value $fine in string at ./overdue_notices.pl line 706.

Error messages from master branch in comparison:
Use of uninitialized value $fine in string at ./overdue_notices.pl line 671.
Use of uninitialized value $fine in string at ./overdue_notices.pl line 671.
Wide character in print at ./overdue_notices.pl line 579.

4) -html with items.content in overdue notice template:

With patch applied:
[...]
Use of uninitialized value in join or string at ./overdue_notices.pl line 502.
Use of uninitialized value in join or string at ./overdue_notices.pl line 502.
issuedatetitlebarcodeauthorbiblionumberdate_due at ./overdue_notices.pl line 503.
Use of uninitialized value in concatenation (.) or string at ./overdue_notices.pl line 504.
Use of uninitialized value in concatenation (.) or string at ./overdue_notices.pl line 504.
;Title;Barcode;Author;Biblio Number (koha internal);; at ./overdue_notices.pl line 504.

On master: no output from overdue_notices.pl

5) In the created html file the headings for columns issue_date and date_due are missing. 

In general the created files and mails looked ok, but lot of error messages and additional created from running overdue_notices.pl.
Comment 3 Katrin Fischer 2011-03-19 17:33:21 UTC
6) Looking at the code I also noticed that country is missing from the following line.

+        my $content = join(";", qw(title name surname address1 address2 zipcode city email itemcount itemsinfo due_date issue_date)) . "\n";
Comment 4 MJ Ray (software.coop) 2011-04-09 09:25:31 UTC
This bug is mentioned in:
Bug 5878 : overdue printing fixes and	improvements http://lists.koha-community.org/pipermail/koha-patches/2011-March/014073.html
Comment 5 Katrin Fischer 2011-06-15 09:11:32 UTC
Patch does not apply any longer. 
I had noted some issues with the patch that should be resolved when rebasing.
Comment 6 Paul Poulain 2011-06-29 19:36:01 UTC
Created attachment 4533 [details] [review]
rebased patch
Comment 7 Katrin Fischer 2011-06-29 20:29:10 UTC
Hi Paul, does the patch also solve the mentioned problems?
Comment 8 Paul Poulain 2011-06-29 20:39:31 UTC
Created attachment 4535 [details] [review]
follow-up fixing perl warnings
Comment 9 Paul Poulain 2011-06-29 20:41:14 UTC
katrin, rebased patch attached, plus a follow-up fixing the perl warns I could reproduce.
If you still have some error, pls specify parameters, I may have tested something not like you.
Comment 10 Sophie MEYNIEUX 2011-07-08 12:09:38 UTC
Created attachment 4601 [details] [review]
only removing some warn left

Trivial patch
Comment 11 Sophie MEYNIEUX 2011-07-08 12:15:02 UTC
Tested with each possible argument separatly
2 warn left in code were generating warning during execution. They were removed in the last follow-up patch
Comment 12 Ian Walls 2011-08-11 17:27:47 UTC
Ideally, I'd like to see the many fixes included in this patch broken into multiple bug reports (and multiple patches), for atomicity of testing.

However, Katrin, if you're willing to repeat your initial round of testing on the newly provided patch (and it's followups) to confirm the issues you noted are resolved, I think we can let this compiled patch stand.  If, however, issues still persist, I would advocate breaking this down into it's component parts, so we can more easily integrate what works without having to wait on the fix for what doesn't.
Comment 13 Katrin Fischer 2011-08-11 20:02:12 UTC
Applied all 3 patches attached to this bug, going through the comments and tests I might on this earlier.

1) Still stands:
-my $itemscontent = join( ',', qw( date_due title barcode author itemnumber )
);
+my $itemscontent = join( ',', qw( issuedate title barcode author biblionumber
date_due) );

I think neither itemnumber nor biblionumber are of any help for patrons. Also having 2 dates can be confusing. I think we should keep the default as simple as possible. Still voting for: 
date_due title author callnumber barcode

Something else: changing the default value  might cause disappointment for installations using the curent default.

2) Is resolved. 

3) <item> syntax in ODUE
<item>"<<biblio.title>>" by <<biblio.author>>, <<items.itemcallnumber>>, Barcode: <<items.barcode>> Gebühr: <fine>EUR</fine></item>

3 a) Create overdues normally using 
overdue_notices.pl -t
OK.

3 b) Create overdues as html notices using
perl overdue_notices.pl -t -html ./

Erorrs output:
Use of uninitialized value in join or string at overdue_notices.pl line 504.
Use of uninitialized value in join or string at overdue_notices.pl line 504.

HTML file looks ok, but errors should be dealt with. 


4) <<items.content>> syntax in ODUE

4 a) 
Create overdues normally using 
overdue_notices.pl -t
OK.

4 b) Create overdues as html notices using
perl overdue_notices.pl -t -html ./

Use of uninitialized value in join or string at overdue_notices.pl line 504.
Use of uninitialized value in join or string at overdue_notices.pl line 504.

Errors should be fixed.

HTML is created. Output overall ok, problems see 5)


5) HTML notices with <<items.content>> 

Some column headings are missing:
	Title	Barcode	Author	Biblio Number (koha internal)	
08/11/2011	Handbuch der Religionspädagogik.	MPL11070003		7	08/09/2011

Missing are: issue_date, barcode, date_due.

Another improvement I would suggest is that the headings should align right. We have no table borders and aligning them center makes the list hard to read.

6) wrong documentation for -html
overdue_notices.pl -h give me following instructions:
-html         <filename>       Output html to file

Should be something like:
-html         <directory>      Output notices as html files to directory
Comment 14 Katrin Fischer 2011-08-11 20:15:30 UTC
7) Umlauts are broken in HTML notice.
I checked by opening the created file in Firefox
Title was: Handbuch der Religionspädagogik.
Comment 15 Katrin Fischer 2011-08-11 20:19:27 UTC
Created attachment 4974 [details]
screenshot for 7)
Comment 16 Ian Walls 2011-08-11 20:30:53 UTC
Okay, as per my early statement and the outcomes of Katrin's testing, I'm going to ask that this patch be broken down into it's constituent features, each filed against its's own bug report, all of which will be linked to this one as bugs that this one "depends on".
Comment 17 Katrin Fischer 2011-08-11 20:32:32 UTC
8) Using perl overdue_notices.pl -t -html ./ -n -list-all

with 
<item>"<<biblio.title>>" by <<biblio.author>>, <<items.itemcallnumber>>,
Barcode: <<items.barcode>> Gebühr: <fine>EUR</fine></item>


HTML file is created, but only FIRST item is printed:

"Handbuch der Religionspädagogik" by , ,

Barcode: MPL11070002 Gebühr: €0,00

For comparison - the screenshot for 7) is made using the same command, but <<items.content>> syntax in the ODUE message.
Comment 18 Paul Poulain 2011-08-31 14:55:42 UTC
oups, this bug is a duplicate of 6030, please test the 6030 instead.
Once 6030 has been applied a small part (for printoverdues.sh) will have to be rewritten
Comment 19 Paul Poulain 2012-01-18 11:23:22 UTC

*** This bug has been marked as a duplicate of bug 6030 ***