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 <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Letters.pm b/C4/Letters.pm
index a981e8a..3ff39b1 100644 (file)
--- 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 (file)
--- a/t/db_dependent/Letters/TemplateToolkit.t
+++ b/t/db_dependent/Letters/TemplateToolkit.t
@@ -18,7 +18,8 @@
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 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(
     (