@@ -, +, @@ digest messages per branch. - There are three libraries - There is a borrower - The borrower is registered at library1 - The borrower has message preference wants_digest set to 1 - The borrower has message preference days_in_advance set to 1 - The content of the letter PREDUEDGST is '<> <>' - There are three items - There is one issue per item - There is one issues at library2 - There are two issues at library3 - The date_due of the issues are set to tomorrow - It is asserted that there are two messages in the message queue after running the script - It is asserted that the item count of the message corresponding to library2 is 1 - It is asserted that the item count of the message corresponding to library3 is 2 - It is asserted that the branchnames are correct. --- .../intranet-tmpl/prog/en/modules/tools/letter.tt | 2 - misc/cronjobs/advance_notices.pl | 96 +++++++++------------- t/db_dependent/cronjobs/advance_notices_digest.t | 45 ++++++++-- 3 files changed, 79 insertions(+), 64 deletions(-) --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt @@ -493,8 +493,6 @@ $(document).ready(function() { [% END %] - [% IF code.search('DGST') %] Warning, this is a template for a Digest, as such, any references to branch data ( e.g. branches.branchname ) will refer to the borrower's home branch. [% END %] - --- a/misc/cronjobs/advance_notices.pl +++ a/misc/cronjobs/advance_notices.pl @@ -218,8 +218,8 @@ warn 'found ' . scalar( @$upcoming_dues ) . ' issues' if $verbose; # hash of borrowernumber to number of items upcoming # for patrons wishing digests only. -my $upcoming_digest; -my $due_digest; +my $upcoming_digest = {}; +my $due_digest = {}; my $dbh = C4::Context->dbh(); my $sth = $dbh->prepare(<<'END_SQL'); @@ -250,8 +250,8 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { if ( $borrower_preferences->{'wants_digest'} ) { # cache this one to process after we've run through all of the items. - $due_digest->{ $upcoming->{borrowernumber} }->{email} = $from_address; - $due_digest->{ $upcoming->{borrowernumber} }->{count}++; + $due_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{email} = $from_address; + $due_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{count}++; } else { my $item = Koha::Items->find( $upcoming->{itemnumber} ); my $letter_type = 'DUE'; @@ -283,8 +283,8 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { if ( $borrower_preferences->{'wants_digest'} ) { # cache this one to process after we've run through all of the items. - $upcoming_digest->{ $upcoming->{borrowernumber} }->{email} = $from_address; - $upcoming_digest->{ $upcoming->{borrowernumber} }->{count}++; + $upcoming_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{email} = $from_address; + $upcoming_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{count}++; } else { my $item = Koha::Items->find( $upcoming->{itemnumber} ); my $letter_type = 'PREDUE'; @@ -342,33 +342,39 @@ SELECT biblio.*, items.*, issues.* AND (TO_DAYS(date_due)-TO_DAYS(NOW()) = ?) END_SQL -send_digests({ - sth => $sth_digest, - digests => $upcoming_digest, - letter_code => 'PREDUEDGST', - get_item_info => sub { - my $params = shift; - $params->{sth}->execute($params->{borrowernumber}, - $params->{borrower_preferences}->{'days_in_advance'}); - return sub { - $params->{sth}->fetchrow_hashref; - }; - } -}); - -send_digests({ - sth => $sth_digest, - digest => $due_digest, - letter_code => 'DUEGST', - get_item_info => sub { - my $params = shift; - $params->{sth}->execute($params->{borrowernumber}, 0); - return sub { - $params->{sth}->fetchrow_hashref; - }; - } -}); - +while (my ($branchcode, $digests) = each %$upcoming_digest) { + send_digests({ + sth => $sth_digest, + digests => $digests, + letter_code => 'PREDUEDGST', + branchcode => $branchcode, + get_item_info => sub { + my $params = shift; + $params->{sth}->execute($params->{borrowernumber}, + $params->{borrower_preferences}->{'days_in_advance'}); + return sub { + $params->{sth}->fetchrow_hashref; + }; + } + }); +} + +while (my ($branchcode, $digests) = each %$due_digest) { + send_digests({ + sth => $sth_digest, + digest => $due_digest, + letter_code => 'DUEGST', + branchcode => $branchcode, + get_item_info => sub { + my $params = shift; + $params->{sth}->execute($params->{borrowernumber}, 0); + return sub { + $params->{sth}->fetchrow_hashref; + }; + } + }); +} + =head1 METHODS =head2 parse_letter @@ -408,24 +414,6 @@ sub parse_letter { ); } -=head2 get_branch_info - -=cut - -sub get_branch_info { - my ( $borrowernumber ) = @_; - - ## Get branch info for borrowers home library. - my $patron = Koha::Patrons->find( $borrowernumber ); - my $branch = $patron->library->unblessed; - my %branch_info; - foreach my $key( keys %$branch ) { - $branch_info{"branches.$key"} = $branch->{$key}; - } - - return %branch_info; -} - =head2 send_digests send_digests({ @@ -491,9 +479,6 @@ sub send_digests { $titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields } ); } - ## Get branch info for borrowers home library. - my %branch_info = get_branch_info( $borrowernumber ); - foreach my $transport ( keys %{ $borrower_preferences->{'transports'} } ) { my $letter = parse_letter( { @@ -502,9 +487,8 @@ sub send_digests { substitute => { count => $count, 'items.content' => $titles, - %branch_info, }, - branchcode => $branch_info{"branches.branchcode"}, + branchcode => $params->{branchcode}, message_transport_type => $transport, } ) --- a/t/db_dependent/cronjobs/advance_notices_digest.t +++ a/t/db_dependent/cronjobs/advance_notices_digest.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 5; use t::lib::TestBuilder; use DateTime; use File::Spec; @@ -113,6 +113,9 @@ my $item1 = $builder->build({ my $item2 = $builder->build({ source => 'Item' }); +my $item3 = $builder->build({ + source => 'Item' +}); my $now = DateTime->now(); my $tomorrow = $now->add(days => 1)->strftime('%F'); @@ -121,7 +124,7 @@ my $issue1 = $builder->build({ value => { date_due => $tomorrow, itemnumber => $item1->{itemnumber}, - branchcode => $library1->{branchcode}, + branchcode => $library2->{branchcode}, borrowernumber => $borrower->{borrowernumber}, returndate => undef } @@ -132,7 +135,16 @@ my $issue2 = $builder->build({ value => { date_due => $tomorrow, itemnumber => $item2->{itemnumber}, - branchcode => $library2->{branchcode}, + branchcode => $library3->{branchcode}, + borrowernumber => $borrower->{borrowernumber}, + returndate => undef + } +}); +my $issue3 = $builder->build({ + source => 'Issue', + value => { + date_due => $tomorrow, + itemnumber => $item3->{itemnumber}, branchcode => $library3->{branchcode}, borrowernumber => $borrower->{borrowernumber}, returndate => undef @@ -160,14 +172,35 @@ $sthmq->execute($borrower->{borrowernumber}); my $messages = $sthmq->fetchall_hashref('message_id'); -is(scalar(keys %$messages), 1, 'There is one message in the queue'); +is(scalar(keys %$messages), 2, 'There are two messages in the queue'); + +my %expected = ( + $library2->{branchname} => { + count => 1, + }, + $library3->{branchname} => { + count => 2, + } + ); + +my %expected_branchnames = ( + $library2->{branchname} => 1, + $library3->{branchname} => 1 +); + +my $i = 0; for my $message (keys %$messages) { $messages->{$message}->{content} =~ /(\d+) (.*)/; my $count = $1; my $branchname = $2; - is ($count, '2', 'Issue count is 2'); - is ($branchname, $library1->{branchname}, 'Branchname is that of borrowers home branch.'); + ok ($expected_branchnames{$branchname}, 'Branchname is that of expected issuing branch.'); + + $expected_branchnames{$branchname} = 0; + + is ($count, $expected{$branchname}->{count}, 'Issue count is ' . $expected{$branchname}->{count}); + + $i++; } $dbh->rollback; --