From 7ef5749fd4e8a2335a91450367f13123ff82fb02 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 5 Sep 2016 09:54:35 +0100 Subject: [PATCH] Bug 17246: Do no support arrayref to define multiple FK Currently you can call GetPreparedLetter like: $prepared_letter = GetPreparedLetter( ( module => 'test', letter_code => 'TEST_HOLD', tables => { reserves => [ $fk1, $fk2 ], }, ) ); It assumes that $fk1 is a borrowernumber and $fk2 a biblionumber. It seems hazardous to do this guess. I suggest to remove this feature and only allow hashref indeed. Test plan: Use different way to generate letters and make sure you do not reach the croak Signed-off-by: Josef Moravec Signed-off-by: Kyle M Hall --- C4/Letters.pm | 5 +---- t/db_dependent/Letters/TemplateToolkit.t | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index a981e8a..3ff39b1 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1570,10 +1570,7 @@ sub _get_tt_params { $object = $module->search( { $pk => $tables->{$table} } )->next(); } else { # Params are mutliple foreign keys - my @values = @{ $tables->{$table} }; - my @keys = @{ $config->{$table}->{fk} }; - my %params = map { $_ => shift(@values) } @keys; - $object = $module->search( \%params )->next(); + croak "Multiple foreign keys (table $table) should be passed using an hashref"; } $params->{ $config->{$table}->{singular} } = $object; } diff --git a/t/db_dependent/Letters/TemplateToolkit.t b/t/db_dependent/Letters/TemplateToolkit.t index 4169556..bdcfb96 100644 --- a/t/db_dependent/Letters/TemplateToolkit.t +++ b/t/db_dependent/Letters/TemplateToolkit.t @@ -18,7 +18,8 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 14; +use Test::More tests => 15; +use Test::Warn; use MARC::Record; @@ -182,12 +183,26 @@ $prepared_letter = GetPreparedLetter( module => 'test', letter_code => 'TEST_HOLD', tables => { - reserves => [ $patron->{borrowernumber}, $biblio->id() ] + reserves => { borrowernumber => $patron->{borrowernumber}, biblionumber => $biblio->id() }, }, ) ); is( $prepared_letter->{content}, $hold->id(), 'Hold object used correctly' ); +eval { + $prepared_letter = GetPreparedLetter( + ( + module => 'test', + letter_code => 'TEST_HOLD', + tables => { + reserves => [ $patron->{borrowernumber}, $biblio->id() ], + }, + ) + ) +}; +my $croak = $@; +like( $croak, qr{^Multiple foreign keys \(table reserves\) should be passed using an hashref.*}, "GetPreparedLetter should not be called with arrayref for multiple FK" ); + # Bug 16942 $prepared_letter = GetPreparedLetter( ( -- 2.1.4