From 2e5b334dfe2de04d23c237a18c93141c74dc35fc Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 28 Dec 2022 10:43:16 -0300 Subject: [PATCH] Bug 14251: Allow use of CSS in discharge letter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The discharge feature relies PDF::FromHTML which explicitly doesn't handle CSS [1]. This patch makes it use the `weasyprint` command-line tool to generate the PDF. This tool handles CSS correctly. To test: 1. Install weasyprint: $ apt install weasyprint 2. Add some style to your DISCHARGE letter. I used: <>

Discharge confirmation

<> certifies that the following borrower:
<> <> (cardnumber: <>)
has returned all items.

3. Have some non-latin chars on the patron name. I picked 'Henry Acevedo' and added 'Δοκιμή' as picked from bug 23589. Only to check no regressions. 4. Enable the 'UseDischarge' syspref 5. Go to Henry's detail page 6. Choose More > Discharge and Generate the discharge => SUCCESS: - Style is applied to the PDF - Greek characters are displayed correctly 7. Run the tests: $ kshell k$ prove -v t/db_dependent/Patron/Borrower_Discharge.t => SUCCESS: The rewritten tests pass! 8. Remove weasyprint: $ apt remove weasyprint 9. Repeat 7 => SUCCESS: Tests pass, relevant test is skipped because of missing weasyprint 10. Sign off :-D [1] https://metacpan.org/pod/PDF::FromHTML#CAVEATS --- Koha/Patron/Discharge.pm | 46 ++++++++------------- t/db_dependent/Patron/Borrower_Discharge.t | 47 +++++++++++++++++----- 2 files changed, 53 insertions(+), 40 deletions(-) diff --git a/Koha/Patron/Discharge.pm b/Koha/Patron/Discharge.pm index 76e04e8533a..f86d4d77eec 100644 --- a/Koha/Patron/Discharge.pm +++ b/Koha/Patron/Discharge.pm @@ -3,6 +3,7 @@ package Koha::Patron::Discharge; use Modern::Perl; use CGI; use File::Temp qw( tmpnam ); +use IPC::Cmd; use Carp qw( carp ); use C4::Templates qw ( gettemplate ); @@ -128,40 +129,27 @@ sub generate_as_pdf { my $html_path = tmpnam() . '.html'; my $pdf_path = tmpnam() . '.pdf'; my $html_content = $tmpl->output; + + # export to HTML open my $html_fh, '>:encoding(utf8)', $html_path; say $html_fh $html_content; close $html_fh; - my $output = eval { require PDF::FromHTML; return; } || $@; - if ($output && $params->{testing}) { - carp $output; - $pdf_path = undef; - } - elsif ($output) { - die $output; + + if ( IPC::Cmd::can_run('weasyprint') ) { + my( $success, $error_message, $full_buf, $stdout_buf, $stderr_buf ) = + IPC::Cmd::run( command => "weasyprint $html_path $pdf_path", verbose => 0 ); + + map {warn $_} @$stderr_buf + if $stderr_buf and scalar @$stderr_buf; + + unless ( $success ) { + warn $error_message; + $pdf_path = undef; + } } else { - my $pdf = PDF::FromHTML->new( encoding => 'utf-8' ); - $pdf->load_file( $html_path ); - - my $ttf = C4::Context->config('ttf'); - if ( $ttf && exists $ttf->{font} ) { - - my $type2path; - foreach my $font ( @{ $ttf->{font} } ) { - $type2path->{ $font->{type} } = $font->{content}; - } - - $pdf->convert( - FontBold => $type2path->{'HB'} || 'HelveticaBold', - FontOblique => $type2path->{'HO'} || 'HelveticaOblique', - FontBoldOblique => $type2path->{'HBO'}|| 'HelveticaBoldOblique', - FontUnicode => $type2path->{'H'} || 'Helvetica', - Font => $type2path->{'H'} || 'Helvetica', - ); - } else { - $pdf->convert(); - } - $pdf->write_file( $pdf_path ); + warn "weasyprint not found!"; + $pdf_path = undef; } return $pdf_path; diff --git a/t/db_dependent/Patron/Borrower_Discharge.t b/t/db_dependent/Patron/Borrower_Discharge.t index 8c742195377..3de4ade2e46 100755 --- a/t/db_dependent/Patron/Borrower_Discharge.t +++ b/t/db_dependent/Patron/Borrower_Discharge.t @@ -15,8 +15,13 @@ # with Koha; if not, see . use Modern::Perl; -use Test::More tests => 19; + +use Test::More tests => 23; + +use Test::MockModule; use Test::Warn; + +use IPC::Cmd qw(can_run); use MARC::Record; use C4::Circulation qw( AddIssue AddReturn ); @@ -114,18 +119,38 @@ is(scalar( Koha::Patron::Discharge::get_pendings ), 1, 'There is a pending disch Koha::Patron::Discharge::discharge( { borrowernumber => $patron->{borrowernumber} } ); is_deeply( [ Koha::Patron::Discharge::get_pendings ], [], 'There is no pending discharge request (second time)'); -# Check if PDF::FromHTML is installed. -my $check = eval { require PDF::FromHTML; }; +SKIP: { + skip "Skipping because weasyprint is not installed", + 1 unless can_run('weasyprint'); -# Tests for if PDF::FromHTML is installed -if ($check) { - isnt( Koha::Patron::Discharge::generate_as_pdf({ borrowernumber => $patron->{borrowernumber} }), undef, "Temporary PDF generated." ); + isnt( + Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->{borrowernumber} } ), + undef, + "Temporary PDF generated." + ); } -# Tests for if PDF::FromHTML is not installed -else { - warning_like { Koha::Patron::Discharge::generate_as_pdf({ borrowernumber => $patron->{borrowernumber}, testing => 1 }) } - [ qr/Can't locate PDF\/FromHTML.pm in \@INC/ ], - "Expected failure because of missing PDF::FromHTML."; + +{ + my $mocked_ipc = Test::MockModule->new('IPC::Cmd'); + + $mocked_ipc->mock( 'run', sub { return 0, 'Some error' } ); + + my $result; + warning_is + { $result = Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->{borrowernumber} } ); } + 'Some error', + 'Failed call to run() prints the generated error'; + + is( $result, undef, 'undef returned if failed run' ); + + $mocked_ipc->mock( 'can_run', undef ); + + warning_is + { $result = Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->{borrowernumber} } ); } + 'weasyprint not found!', + 'Expected failure because of missing weasyprint'; + + is( $result, undef, 'undef returned if missing weasyprint' ); } # FIXME Should be a Koha::Object object -- 2.34.1