Bug 17969: Refactor the way <<items.content>> is generated

<<items.content>> is generated 4x in advance_notices.pl and once in
overdue_notices.pl
It would be better to have it in C4::Letters.
It will enforce the fact that it already has the same behavior, make it
testable and reusable.

Test plan:
Use the <<items.content>> tag for advance and overdue notices.
The generated notices must be the same as before this patch.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart 2017-01-19 20:11:21 +01:00
parent bc09af634c
commit e63eee05a0
4 changed files with 61 additions and 21 deletions

View File

@ -1632,6 +1632,25 @@ sub _get_tt_params {
return $params;
}
sub get_item_content {
my ( $params ) = @_;
my $item = $params->{item};
my $dateonly = $params->{dateonly} || 0;
my $item_content_fields = $params->{item_content_fields} || [];
return unless $item;
my @item_info = map {
$_ =~ /^date|date$/
? eval {
output_pref(
{ dt => dt_from_string( $item->{$_} ), dateonly => $dateonly } );
}
: $item->{$_}
|| ''
} @$item_content_fields;
return join( "\t", @item_info ) . "\n";
}
1;
__END__

View File

@ -258,8 +258,7 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) {
$sth->execute($upcoming->{'borrowernumber'},$upcoming->{'itemnumber'},'0');
my $titles = "";
while ( my $item_info = $sth->fetchrow_hashref()) {
my @item_info = map { $_ =~ /^date|date$/ ? format_date($item_info->{$_}) : $item_info->{$_} || '' } @item_content_fields;
$titles .= join("\t",@item_info) . "\n";
$titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields } );
}
## Get branch info for borrowers home library.
@ -292,8 +291,7 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) {
$sth->execute($upcoming->{'borrowernumber'},$upcoming->{'itemnumber'},$borrower_preferences->{'days_in_advance'});
my $titles = "";
while ( my $item_info = $sth->fetchrow_hashref()) {
my @item_info = map { $_ =~ /^date|date$/ ? format_date($item_info->{$_}) : $item_info->{$_} || '' } @item_content_fields;
$titles .= join("\t",@item_info) . "\n";
$titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields } );
}
## Get branch info for borrowers home library.
@ -358,8 +356,7 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$upcoming_digest ) {
$sth->execute($borrowernumber,$borrower_preferences->{'days_in_advance'});
my $titles = "";
while ( my $item_info = $sth->fetchrow_hashref()) {
my @item_info = map { $_ =~ /^date|date$/ ? format_date($item_info->{$_}) : $item_info->{$_} || '' } @item_content_fields;
$titles .= join("\t",@item_info) . "\n";
$titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields } );
}
## Get branch info for borrowers home library.
@ -415,8 +412,7 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$due_digest ) {
$sth->execute($borrowernumber,'0');
my $titles = "";
while ( my $item_info = $sth->fetchrow_hashref()) {
my @item_info = map { $_ =~ /^date|date$/ ? format_date($item_info->{$_}) : $item_info->{$_} || '' } @item_content_fields;
$titles .= join("\t",@item_info) . "\n";
$titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields } );
}
## Get branch info for borrowers home library.
@ -497,12 +493,6 @@ sub parse_letter {
);
}
sub format_date {
my $date_string = shift;
my $dt=dt_from_string($date_string);
return output_pref($dt);
}
=head2 get_branch_info
=cut

View File

@ -663,11 +663,8 @@ END_SQL
last;
}
$j++;
my @item_info = map { $_ =~ /^date|date$/ ?
eval { output_pref( { dt => dt_from_string( $item_info->{$_} ), dateonly => 1 } ); }
:
$item_info->{$_} || '' } @item_content_fields;
$titles .= join("\t", @item_info) . "\n";
$titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields, dateonly => 1 } );
$itemcount++;
push @items, $item_info;
}

View File

@ -18,7 +18,7 @@
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use Test::More tests => 75;
use Test::More tests => 76;
use Test::MockModule;
use Test::Warn;
@ -652,5 +652,39 @@ subtest 'SendQueuedMessages' => sub {
status => 'sent'
})->next()->to_address();
is( $sms_message_address, '5555555555@kidclamp.rocks', 'SendQueuedMessages populates the to address correctly for SMS by email' );
};
subtest 'get_item_content' => sub {
plan tests => 2;
t::lib::Mocks::mock_preference('dateformat', 'metric');
t::lib::Mocks::mock_preference('timeformat', '24hr');
my @items = (
{date_due => '2041-01-01 12:34', title => 'a first title', barcode => 'a_first_barcode', author => 'a_first_author', itemnumber => 1 },
{date_due => '2042-01-02 23:45', title => 'a second title', barcode => 'a_second_barcode', author => 'a_second_author', itemnumber => 2 },
);
my @item_content_fields = qw( date_due title barcode author itemnumber );
my $items_content;
for my $item ( @items ) {
$items_content .= C4::Letters::get_item_content( { item => $item, item_content_fields => \@item_content_fields } );
}
my $expected_items_content = <<EOF;
01/01/2041 12:34\ta first title\ta_first_barcode\ta_first_author\t1
02/01/2042 23:45\ta second title\ta_second_barcode\ta_second_author\t2
EOF
is( $items_content, $expected_items_content, 'get_item_content should return correct items info with time (default)' );
$items_content = q||;
for my $item ( @items ) {
$items_content .= C4::Letters::get_item_content( { item => $item, item_content_fields => \@item_content_fields, dateonly => 1, } );
}
$expected_items_content = <<EOF;
01/01/2041\ta first title\ta_first_barcode\ta_first_author\t1
02/01/2042\ta second title\ta_second_barcode\ta_second_author\t2
EOF
is( $items_content, $expected_items_content, 'get_item_content should return correct items info without time (if dateonly => 1)' );
};