Bug 9210 - Automatic carriage return are missing in print letters generated for holds
Summary: Automatic carriage return are missing in print letters generated for holds
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Sophie MEYNIEUX
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-04 17:00 UTC by Sophie MEYNIEUX
Modified: 2015-06-04 23:23 UTC (History)
6 users (show)

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


Attachments
Bug 9210 : letters generated for holds are missing wrapping for words and lines. (1.08 KB, patch)
2012-12-04 17:08 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review
Bug 9210 : letters generated for holds are missing wrapping for words and lines. (1.13 KB, patch)
2012-12-07 16:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9210 : letters generated for holds are missing wrapping for words and lines. (1.17 KB, patch)
2012-12-21 13:46 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 9210 : letters generated for holds are missing wrapping for words and lines. (1.57 KB, patch)
2013-01-03 16:15 UTC, Paul Poulain
Details | Diff | Splinter Review
html file used to test pdf generation (2.46 KB, text/html)
2013-03-19 15:40 UTC, Sophie MEYNIEUX
Details
Bug 9210 : letters generated for holds are missing wrapping for words and lines. (1.66 KB, patch)
2014-01-15 11:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
pdf generated without the patch (1.58 KB, application/pdf)
2014-01-15 12:00 UTC, Jonathan Druart
Details
pdf generated with the patch (2.36 KB, application/pdf)
2014-01-15 12:00 UTC, Jonathan Druart
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sophie MEYNIEUX 2012-12-04 17:00:39 UTC
When you generate print letters (pdf) for holds with gather_print_notices.pl, lines that are too long are truncated and not automatically wrapped to another line.

This patch modify styles in the template used for generation of pdf files.
Comment 1 Sophie MEYNIEUX 2012-12-04 17:08:40 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-12-07 16:16:40 UTC Comment hidden (obsolete)
Comment 3 Paul Poulain 2012-12-21 13:46:22 UTC Comment hidden (obsolete)
Comment 4 Paul Poulain 2012-12-21 13:47:12 UTC
QA comment: tiny patch, css only, no side effect risk

passed QA
Comment 5 Jared Camins-Esakov 2012-12-22 21:30:13 UTC
Could you please provide a detailed test plan for this patch? The description talks about gather_print_notices.pl producing a PDF, but it doesn't seem to do that.
Comment 6 Jared Camins-Esakov 2012-12-27 23:43:08 UTC
Marking as "Failed QA" until I have an answer about the test plan.
Comment 7 Paul Poulain 2013-01-03 16:15:04 UTC Comment hidden (obsolete)
Comment 8 Paul Poulain 2013-01-03 16:15:30 UTC
test plan now attached to the bug
Comment 9 Jared Camins-Esakov 2013-01-04 13:39:08 UTC
The script misc/cronjobs/printoverdues.sh does not work at all for me:
jcamins@kohadev:~/kohaclone$ sh -v misc/cronjobs/printoverdues.sh /tmp                                                                                          [9/1955]
#!/bin/bash

print_usage(){
    echo "$0 : generates PDF files from html files in directorys and prints them";
    echo "usage :";
    echo "$0 directory [css [printer_host [printername]]]"
    echo " - directory      directory to use to apply html2pdf transform";
    echo " - css            css file to apply to html ";
    echo " - printer_host   Network Name or IP of the printer (port possibly included) ";
    echo " - printer_name   printername ";
    echo "Note that css printerhost and printername are optional parameters ";
    echo "Note that this script uses xhtml2pdf command ";
    echo "    xhtml2pdf command comes with pisa (a python library)
              To install you need setuptools library for python
              then type easy_install pisa ";

    exit 1;
}
if [ $# -lt 1 ]
then
    print_usage
fi
if [[ ! -d $1  ]]
then
    echo "$1 : directory expected";
    exit 1;
fi
misc/cronjobs/printoverdues.sh: 23: misc/cronjobs/printoverdues.sh: [[: not found
if [[ -n $2 && -f $2 ]]
then
    optpisa="--css $2";
fi
misc/cronjobs/printoverdues.sh: 28: misc/cronjobs/printoverdues.sh: [[: not found
if [[ -n $3 ]]
then
    optprinter="-h $3";
fi
misc/cronjobs/printoverdues.sh: 32: misc/cronjobs/printoverdues.sh: [[: not found
if [[ -n $4 ]]
then
    optprinter="$optprinter -d $4";
fi
misc/cronjobs/printoverdues.sh: 36: misc/cronjobs/printoverdues.sh: [[: not found
for i in $1/*.html
do
    xhtml2pdf $optpisa  $i;
done

if [[ -n $optprinter ]]
then
    lp $optprinter  $1/*.pdf;
fi
misc/cronjobs/printoverdues.sh: 45: misc/cronjobs/printoverdues.sh: [[: not found

tar cvfz $directory`date "+%Y%m%d"`.tar.gz  $directory;
tar: Cowardly refusing to create an empty archive
Try `tar --help' or `tar --usage' for more information.

A fix to the syntax so that the script works on current master will be needed before I can push this patch.
Comment 10 Paul Poulain 2013-01-18 10:25:37 UTC
Sorry Jared, but I don't understand your last comment. What's your problem ?
Comment 11 Jared Camins-Esakov 2013-01-18 12:11:16 UTC
(In reply to comment #10)
> Sorry Jared, but I don't understand your last comment. What's your problem ?

There's a syntax error in the printoverdues.sh script, on line 45, and I won't push a patch that can't be tested.
Comment 12 Jared Camins-Esakov 2013-01-24 15:50:30 UTC
Okay, the problem was that I was using sh when printoverdues.sh is actually a bash script.

However, the patch results in a major regression. Instead of having a formatted hold notice with one line going off the edge of the paper, all the text is run together: http://kohadev.cpbibliography.com/holdnotices-2013-01-24.pdf
Comment 13 Sophie MEYNIEUX 2013-02-20 13:33:45 UTC
Jared, I can't see your generated pdf file. Could you attach it and also the html file, please.
Comment 14 Sophie MEYNIEUX 2013-03-19 15:39:06 UTC
Jared, with the html file you send me, I can't generate a formatted pdf file even without the patch.

This patch only deals with wrapping long lines, not carriage return (title is confusing)
Comment 15 Sophie MEYNIEUX 2013-03-19 15:40:35 UTC
Created attachment 16414 [details]
html file used to test pdf generation

This file is mentionned in comments
Comment 16 Jonathan Druart 2014-01-15 11:57:18 UTC
I don't know why this one is still needs signoff. It has been signed off twice.
Comment 17 Jonathan Druart 2014-01-15 11:58:04 UTC
QA comment:
The patch works great, see the 2 pdf I will send.
Marked as Passed QA.
Comment 18 Jonathan Druart 2014-01-15 11:59:03 UTC
Created attachment 24394 [details] [review]
Bug 9210 : letters generated for holds are missing wrapping for words and lines.

This commit adds wrapping styles to print-notices.tt template

Test plan:
  * place a hold on a title with a very very long title or author
  * mark it as "waiting for pickup" by doing a checkin
  * go to command line interface, and run
  * misc/cronjobs/gather_print_notices.pl /tmp
  * misc/cronjobs/printoverdues.sh /tmp
  * if your title/author is long enough, it's splitted on 2 lines
    with this patch (it is not before this patch)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 19 Jonathan Druart 2014-01-15 12:00:16 UTC
Created attachment 24395 [details]
pdf generated without the patch
Comment 20 Jonathan Druart 2014-01-15 12:00:41 UTC
Created attachment 24396 [details]
pdf generated with the patch
Comment 21 Galen Charlton 2014-01-15 16:01:08 UTC
Pushed to master.  Thanks, Sophie!
Comment 22 Fridolin Somers 2014-03-18 07:58:37 UTC
Patch pushed to 3.14.x, will be in 3.14.5
Comment 23 Chris Cormack 2014-03-24 04:44:35 UTC
Pushed to 3.12.x will be in 3.12.12