Bugzilla – Attachment 65567 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.83 KB, created by
Tomás Cohen Arazi (tcohen)
on 2017-08-07 15:14:55 UTC
(
hide
)
Description:
Bug 17969: Refactor the way <<items.content>> is generated
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2017-08-07 15:14:55 UTC
Size:
7.83 KB
patch
obsolete
>From 5955a1c7bf84a53980cbc3b32271821c1b040805 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 >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > ><<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> >--- > C4/Letters.pm | 19 +++++++++++++++++++ > misc/cronjobs/advance_notices.pl | 18 ++++-------------- > misc/cronjobs/overdue_notices.pl | 7 ++----- > t/db_dependent/Letters.t | 36 +++++++++++++++++++++++++++++++++++- > 4 files changed, 60 insertions(+), 20 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index de8253b..a3821ea 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -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__ >diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl >index 0475ee0..73697b8 100755 >--- a/misc/cronjobs/advance_notices.pl >+++ b/misc/cronjobs/advance_notices.pl >@@ -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 >diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl >index 7497381..a1b477c 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 cbb2bec..2c96e23 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 => 75; >+use Test::More tests => 76; > use Test::MockModule; > use Test::Warn; > >@@ -649,5 +649,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)' ); > }; >-- >2.7.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