QA comment: tiny patch, css only, no side effect risk

passed QA

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.

Marking as "Failed QA" until I have an answer about the test plan.

test plan now attached to the bug

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.

Sorry Jared, but I don't understand your last comment. What's your problem ?

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

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

Jared, I can't see your generated pdf file. Could you attach it and also the html file, please.

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)

Created attachment 16414 [details]
html file used to test pdf generation

This file is mentionned in comments

I don't know why this one is still needs signoff. It has been signed off twice.

QA comment:
The patch works great, see the 2 pdf I will send.
Marked as Passed QA.

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>

Created attachment 24395 [details]
pdf generated without the patch

Created attachment 24396 [details]
pdf generated with the patch

Pushed to master.  Thanks, Sophie!

Patch pushed to 3.14.x, will be in 3.14.5

Pushed to 3.12.x will be in 3.12.12