From 1dcd988f186abe85718a2ad684760f37fb6c1e4c Mon Sep 17 00:00:00 2001 From: Martin Stenberg Date: Wed, 16 Dec 2015 13:35:06 +0100 Subject: [PATCH] Bug 15364: accountlines variable in letters Add accountlines variable to letter templates for advance_notices and overdue_notices. Test plan ========= 1. Edit the overdue notice template to look like this: <> - <> Total: <> 2. Make sure at least one patron has at least one accountline, if not, create one by checking out an item with a past due date. 3. run bin/cronjobs/overdue_notices.pl and check the generated mail content 4. run bin/cronjobs/advance_notices.pl and check the generated mail content 5. generated mail content should look something like this: $5 - Fine $10 - Rent Total: $15 --- C4/Letters.pm | 1 + C4/Overdues.pm | 36 +++++++++++++++++++++- misc/cronjobs/advance_notices.pl | 65 ++++++++++++++++++++++++++++++++++++++++ misc/cronjobs/overdue_notices.pl | 17 +++++++++-- t/db_dependent/Letters.t | 62 ++++++++++++++++++++++++++++---------- tools/letter.pl | 1 + 6 files changed, 164 insertions(+), 18 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index bc60bc3..25dd3f5 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -758,6 +758,7 @@ sub _parseletter_sth { # broke things for the rest of us. prepare_cached is a better # way to cache statement handles anyway. my $query = + ($table eq 'accountlines' ) ? "SELECT * FROM $table WHERE borrowernumber = ? AND amountoutstanding > 0" : ($table eq 'biblio' ) ? "SELECT * FROM $table WHERE biblionumber = ?" : ($table eq 'biblioitems' ) ? "SELECT * FROM $table WHERE biblionumber = ?" : ($table eq 'items' ) ? "SELECT * FROM $table WHERE itemnumber = ?" : diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 72c0704..d4fd06b 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -1004,22 +1004,56 @@ sub parse_overdues_letter { # if active currency isn't correct ISO code fallback to sprintf $item->{'fine'} = sprintf('%.2f', $fine) unless $item->{'fine'}; + $item->{'amountoutstanding'} = currency_format($currency_format, $item->{'amountoutstanding'}, FMT_SYMBOL); + # if active currency isn't correct ISO code fallback to sprintf + $item->{'amountoutstanding'} = sprintf('%.2f', $item->{'amountoutstanding'}) unless $item->{'amountoutstanding'}; + + my $accountline = { + 'amountoutstanding' => $item->{'amountoutstanding'}, + 'description' => $item->{'description'}, + }; + push @item_tables, { 'biblio' => $item->{'biblionumber'}, 'biblioitems' => $item->{'biblionumber'}, 'items' => $item, + 'accountlines' => $accountline, 'issues' => $item->{'itemnumber'}, }; } } + my $amountoutstanding_total = 0; + my @accountlines_tables; + if ( my $i = $params->{'accountlines'} ) { + foreach my $accountline (@$i) { + $amountoutstanding_total += $accountline->{'amountoutstanding'}; + + $accountline->{'amountoutstanding'} = currency_format($currency_format, $accountline->{'amountoutstanding'}, FMT_SYMBOL); + # if active currency isn't correct ISO code fallback to sprintf + $accountline->{'amountoutstanding'} = sprintf('%.2f', $accountline->{'amountoutstanding'}) unless $accountline->{'amountoutstanding'}; + $accountline->{'amount'} = currency_format($currency_format, $accountline->{'amount'}, FMT_SYMBOL); + # if active currency isn't correct ISO code fallback to sprintf + $accountline->{'amount'} = sprintf('%.2f', $accountline->{'amount'}) unless $accountline->{'amount'}; + + push @accountlines_tables, { + 'accountlines' => $accountline + }; + } + } + $amountoutstanding_total = currency_format($currency_format, $amountoutstanding_total, FMT_SYMBOL); + # if active currency isn't correct ISO code fallback to sprintf + $amountoutstanding_total = sprintf('%.2f', $amountoutstanding_total) unless $amountoutstanding_total; + + $substitute->{'accountlines.total'} = $amountoutstanding_total; + return C4::Letters::GetPreparedLetter ( module => 'circulation', letter_code => $params->{'letter_code'}, branchcode => $params->{'branchcode'}, tables => \%tables, substitute => $substitute, - repeat => { item => \@item_tables }, + repeat => { item => \@item_tables, accountline => \@accountlines_tables}, message_transport_type => $params->{message_transport_type}, ); } diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl index 16800f0..65cae26 100755 --- a/misc/cronjobs/advance_notices.pl +++ b/misc/cronjobs/advance_notices.pl @@ -43,6 +43,7 @@ use warnings; use Getopt::Long; use Pod::Usage; use Data::Dumper; +use Locale::Currency::Format 1.28; BEGIN { # find Koha's Perl modules # test carefully before changing this @@ -55,6 +56,7 @@ use C4::Letters; use C4::Members; use C4::Members::Messaging; use C4::Overdues; +use C4::Budgets qw(GetCurrency); use Koha::DateUtils; use C4::Log; @@ -235,6 +237,18 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { my $from_address = $upcoming->{branchemail} || $admin_adress; + my $sth2 = $dbh->prepare(' + SELECT * + FROM accountlines + WHERE borrowernumber = ? AND amountoutstanding > 0 + '); + $sth2->execute($upcoming->{'borrowernumber'}); + my @accountlines = (); + while ( my $accountline = $sth2->fetchrow_hashref() ) { + push(@accountlines, $accountline); + } + $sth2->finish; + my $borrower_preferences; if ( 0 == $upcoming->{'days_until_due'} ) { # This item is due today. Send an 'item due' message. @@ -265,6 +279,7 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { itemnumber => $upcoming->{'itemnumber'}, substitute => { 'items.content' => $titles }, message_transport_type => $transport, + accountlines => \@accountlines, } ) or warn "no letter of type '$letter_type' found. Please see sample_notices.sql"; push @letters, $letter if $letter; @@ -299,6 +314,7 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { itemnumber => $upcoming->{'itemnumber'}, substitute => { 'items.content' => $titles }, message_transport_type => $transport, + accountlines => \@accountlines, } ) or warn "no letter of type '$letter_type' found. Please see sample_notices.sql"; push @letters, $letter if $letter; @@ -342,6 +358,18 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$upcoming_digest ) { my $count = $digest->{count}; my $from_address = $digest->{email}; + my $sth2 = $dbh->prepare(' + SELECT * + FROM accountlines + WHERE borrowernumber = ? AND amountoutstanding > 0 + '); + $sth2->execute($borrowernumber); + my @accountlines = (); + while ( my $accountline = $sth2->fetchrow_hashref() ) { + push(@accountlines, $accountline); + } + $sth2->finish; + my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, message_name => 'advance_notice' } ); next PATRON unless $borrower_preferences; # how could this happen? @@ -371,6 +399,7 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$upcoming_digest ) { }, branchcode => $branch_info{"branches.branchcode"}, message_transport_type => $transport, + accountlines => \@accountlines, } ) or warn "no letter of type '$letter_type' found. Please see sample_notices.sql"; @@ -401,6 +430,18 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$due_digest ) { my $count = $digest->{count}; my $from_address = $digest->{email}; + my $sth2 = $dbh->prepare(' + SELECT * + FROM accountlines + WHERE borrowernumber = ? AND amountoutstanding > 0 + '); + $sth2->execute($borrowernumber); + my @accountlines = (); + while ( my $accountline = $sth2->fetchrow_hashref() ) { + push(@accountlines, $accountline); + } + $sth2->finish; + my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, message_name => 'item_due' } ); next PATRON unless $borrower_preferences; # how could this happen? @@ -428,6 +469,7 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$due_digest ) { }, branchcode => $branch_info{"branches.branchcode"}, message_transport_type => $transport, + accountlines => \@accountlines, } ) or warn "no letter of type '$letter_type' found. Please see sample_notices.sql"; @@ -479,12 +521,35 @@ sub parse_letter { $table_params{'biblioitems'} = $p; } + my $currencies = GetCurrency(); + my $currency_format = $currencies->{currency} if defined($currencies); + + my $amountoutstanding_total = 0; + my @accountlines_tables = (); + foreach my $accountline (@{$params->{'accountlines'}}) { + $amountoutstanding_total += $accountline->{'amountoutstanding'}; + + $accountline->{'amountoutstanding'} = currency_format($currency_format, $accountline->{'amountoutstanding'}, FMT_SYMBOL); + # if active currency isn't correct ISO code fallback to sprintf + $accountline->{'amountoutstanding'} = sprintf('%.2f', $accountline->{'amountoutstanding'}) unless $accountline->{'amountoutstanding'}; + + push @accountlines_tables, { + 'accountlines' => $accountline + }; + } + $amountoutstanding_total = currency_format($currency_format, $amountoutstanding_total, FMT_SYMBOL); + # if active currency isn't correct ISO code fallback to sprintf + $amountoutstanding_total = sprintf('%.2f', $amountoutstanding_total) unless $amountoutstanding_total; + + $params->{'substitute'}->{'accountlines.total'} = $amountoutstanding_total; + return C4::Letters::GetPreparedLetter ( module => 'circulation', letter_code => $params->{'letter_code'}, branchcode => $table_params{'branches'}, substitute => $params->{'substitute'}, tables => \%table_params, + repeat => {accountline => \@accountlines_tables}, message_transport_type => $params->{message_transport_type}, ); } diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl index 98b0a25..d05eb9c 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -447,12 +447,13 @@ foreach my $branchcode (@branches) { $verbose and warn sprintf "branchcode : '%s' using %s\n", $branchcode, $admin_email_address; my $sth2 = $dbh->prepare( <<"END_SQL" ); -SELECT biblio.*, items.*, issues.*, biblioitems.itemtype, TO_DAYS($date)-TO_DAYS(date_due) AS days_overdue, branchname - FROM issues,items,biblio, biblioitems, branches b +SELECT biblio.*, items.*, issues.*, accountlines.*, biblioitems.itemtype, TO_DAYS($date)-TO_DAYS(date_due) AS days_overdue, branchname + FROM issues,items,biblio, biblioitems, accountlines, branches b WHERE items.itemnumber=issues.itemnumber AND biblio.biblionumber = items.biblionumber AND b.branchcode = items.homebranch AND biblio.biblionumber = biblioitems.biblionumber + AND accountlines.itemnumber = items.itemnumber AND issues.borrowernumber = ? END_SQL @@ -670,6 +671,17 @@ END_SQL @message_transport_types = @{ GetOverdueMessageTransportTypes( q{}, $overdue_rules->{categorycode}, $i) } unless @message_transport_types; + $sth2 = $dbh->prepare(' + SELECT * + FROM accountlines + WHERE borrowernumber = ? AND amountoutstanding > 0 + '); + $sth2->execute($borrowernumber); + my @accountlines = (); + while ( my $accountline = $sth2->fetchrow_hashref() ) { + push(@accountlines, $accountline); + } + $sth2->finish; my $print_sent = 0; # A print notice is not yet sent for this patron for my $mtt ( @message_transport_types ) { @@ -679,6 +691,7 @@ END_SQL borrowernumber => $borrowernumber, branchcode => $branchcode, items => \@items, + accountlines => \@accountlines, substitute => { # this appears to be a hack to overcome incomplete features in this code. bib => $branch_details->{'branchname'}, # maybe 'bib' is a typo for 'lib'? 'items.content' => $titles, diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 2e0d854..de23ac3 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -83,6 +83,13 @@ $dbh->do(q| $mtts = C4::Letters::GetMessageTransportTypes(); is_deeply( $mtts, ['email', 'phone', 'print', 'sms'], 'GetMessageTransportTypes returns all values' ); +$dbh->do(q| + INSERT INTO accountlines( amountoutstanding, description, borrowernumber) VALUES (5, 'Rent', |.$borrowernumber.q|) +|); +$dbh->do(q| + INSERT INTO accountlines( amountoutstanding, description, borrowernumber) VALUES (5, 'Fine', |.$borrowernumber.q|) +|); + # EnqueueLetter is( C4::Letters::EnqueueLetter(), undef, 'EnqueueLetter without argument returns undef' ); @@ -151,7 +158,10 @@ URL: <> The following item(s) is/are currently <>: - <>. <>, Barcode: <> +<>, Barcode: <> + +Accountlines: +<> - <> Thank-you for your prompt attention to this matter. Don't forget your date of birth: <>. @@ -245,14 +255,32 @@ my $tables = { my $substitute = { status => 'overdue', }; -my $repeat = [ - { - itemcallnumber => 'my callnumber1', - barcode => '1234', +my $repeat_items = [ + { items => + { + itemcallnumber => 'my callnumber1', + barcode => '1234', + }, }, - { - itemcallnumber => 'my callnumber2', - barcode => '5678', + { items => + { + itemcallnumber => 'my callnumber2', + barcode => '5678', + }, + } +]; +my $repeat_accountlines = [ + { accountlines => + { + amountoutstanding => 5, + description => 'Rent', + }, + }, + { accountlines => + { + amountoutstanding => 5, + description => 'Fine', + }, }, ]; my $prepared_letter = GetPreparedLetter(( @@ -261,7 +289,7 @@ my $prepared_letter = GetPreparedLetter(( letter_code => 'my code', tables => $tables, substitute => $substitute, - repeat => $repeat, + repeat => {item => $repeat_items, accountline => $repeat_accountlines}, )); my $branch = GetBranchDetail($library->{branchcode}); my $my_title_letter = qq|$branch->{branchname} - $substitute->{status}|; @@ -274,8 +302,12 @@ URL: http://thisisatest.com The following item(s) is/are currently $substitute->{status}: - 1. $repeat->[0]->{itemcallnumber}, Barcode: $repeat->[0]->{barcode} - 2. $repeat->[1]->{itemcallnumber}, Barcode: $repeat->[1]->{barcode} +$repeat_items->[0]->{items}->{itemcallnumber}, Barcode: $repeat_items->[0]->{items}->{barcode} +$repeat_items->[1]->{items}->{itemcallnumber}, Barcode: $repeat_items->[1]->{items}->{barcode} + +Accountlines: +$repeat_accountlines->[0]->{accountlines}->{amountoutstanding} - $repeat_accountlines->[0]->{accountlines}->{description} +$repeat_accountlines->[1]->{accountlines}->{amountoutstanding} - $repeat_accountlines->[1]->{accountlines}->{description} Thank-you for your prompt attention to this matter. Don't forget your date of birth: | . output_pref({ dt => $date, dateonly => 1 }) . q|. @@ -290,7 +322,7 @@ $prepared_letter = GetPreparedLetter(( letter_code => 'my code', tables => $tables, substitute => $substitute, - repeat => $repeat, + repeat => {item => $repeat_items, accountline => $repeat_accountlines}, message_transport_type => 'sms', )); $my_content_letter = qq|This is a SMS for an $substitute->{status}|; @@ -303,7 +335,7 @@ $prepared_letter = GetPreparedLetter(( letter_code => 'test_date', tables => $tables, substitute => $substitute, - repeat => $repeat, + repeat => {item => $repeat_items, accountline => $repeat_accountlines}, )); is( $prepared_letter->{content}, q|This one only contains the date: | . output_pref({ dt => $date, dateonly => 1 }) . q|.|, 'dateonly test 1' ); @@ -314,7 +346,7 @@ $prepared_letter = GetPreparedLetter(( letter_code => 'test_date', tables => $tables, substitute => $substitute, - repeat => $repeat, + repeat => {item => $repeat_items, accountline => $repeat_accountlines}, )); is( $prepared_letter->{content}, q|This one only contains the date: | . output_pref({ dt => $date, dateonly => 1 }) . q|.|, 'dateonly test 2' ); @@ -325,7 +357,7 @@ $prepared_letter = GetPreparedLetter(( letter_code => 'test_date', tables => $tables, substitute => $substitute, - repeat => $repeat, + repeat => {item => $repeat_items, accountline => $repeat_accountlines}, )); is( $prepared_letter->{content}, q|This one only contains the date: | . output_pref({ dt => $date, dateonly => 1 }) . q|.|, 'dateonly test 3' ); diff --git a/tools/letter.pl b/tools/letter.pl index f4173dc..d663e59 100755 --- a/tools/letter.pl +++ b/tools/letter.pl @@ -225,6 +225,7 @@ sub add_form { {value => 'items.content', text => 'items.content'}, {value => 'items.fine', text => 'items.fine'}, add_fields('borrowers'); + push @{$field_selection}, add_fields('accountlines'); if ($module eq 'circulation') { push @{$field_selection}, add_fields('opac_news'); -- 2.6.3