Bugzilla – Attachment 59414 Details for
Bug 17969
Refactor the way the items.content placeholder is generated
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17969: Refactor the way <<items.content>> is generated
Bug-17969-Refactor-the-way-itemscontent-is-generat.patch (text/plain), 7.65 KB, created by
Jonathan Druart
on 2017-01-23 14:02:09 UTC
(
hide
)
Description:
Bug 17969: Refactor the way <<items.content>> is generated
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2017-01-23 14:02:09 UTC
Size:
7.65 KB
patch
obsolete
>From 71170867a6490988ab9b1861ed2df7c1aa83a7a7 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Thu, 19 Jan 2017 20:11:21 +0100 >Subject: [PATCH] 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. >--- > C4/Letters.pm | 19 +++++++++++++++++++ > misc/cronjobs/advance_notices.pl | 18 ++++-------------- > misc/cronjobs/overdue_notices.pl | 7 ++----- > t/db_dependent/Letters.t | 38 +++++++++++++++++++++++++++++++++++++- > 4 files changed, 62 insertions(+), 20 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 3ff39b1..e463f85 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -1585,6 +1585,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__ >diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl >index 855492e..d0036c4 100755 >--- a/misc/cronjobs/advance_notices.pl >+++ b/misc/cronjobs/advance_notices.pl >@@ -255,8 +255,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. >@@ -289,8 +288,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. >@@ -355,8 +353,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. >@@ -412,8 +409,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. >@@ -492,12 +488,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 >diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl >index 0aaf22a..7c15cb8 100755 >--- a/misc/cronjobs/overdue_notices.pl >+++ b/misc/cronjobs/overdue_notices.pl >@@ -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; > } >diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t >index 8219e5b..a1ccd66 100644 >--- a/t/db_dependent/Letters.t >+++ b/t/db_dependent/Letters.t >@@ -18,7 +18,7 @@ > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >-use Test::More tests => 79; >+use Test::More tests => 80; > use Test::MockModule; > use Test::Warn; > >@@ -489,3 +489,39 @@ is($err2, 1, "Successfully sent serial notification"); > is($mail{'To'}, 'john.smith@test.de', "mailto correct in sent serial notification"); > is($mail{'Message'}, 'Silence in the library,'.$subscriptionid.',No. 0', 'Serial notification text constructed successfully'); > } >+ >+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)' ); >+ >+}; >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17969
:
59414
|
61637
|
63976
|
65364
|
65567
|
65568
|
65684
|
65685