Bug 19809: Re-allow to call Koha::Objects::find in list context

and remove 'scalar' keyword in calls where it's not needed.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 05d5910..f2bf8bd 100644 (file)
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -830,7 +830,7 @@ sub CheckReserves {
                     next if ( ($hold_fulfillment_policy eq 'holdgroup') && (!$library->validate_hold_sibling({branchcode => $res->{branchcode}})) );
                     next if ( ($hold_fulfillment_policy eq 'homebranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) );
                     next if ( ($hold_fulfillment_policy eq 'holdingbranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) );
-                    next unless $item->can_be_transferred( { to => scalar Koha::Libraries->find( $res->{branchcode} ) } );
+                    next unless $item->can_be_transferred( { to => Koha::Libraries->find( $res->{branchcode} ) } );
                     $priority = $res->{'priority'};
                     $highest  = $res;
                     last if $local_hold_match;

diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm
index e442787..daefee5 100644 (file)
--- a/C4/SIP/ILS/Transaction/Hold.pm
+++ b/C4/SIP/ILS/Transaction/Hold.pm
@@ -60,7 +60,7 @@ sub do_hold {
         $self->ok(0);
         return $self;
     }
-    unless ( $item->can_be_transferred( { to => scalar Koha::Libraries->find( $branch ) } ) ) {
+    unless ( $item->can_be_transferred( { to => Koha::Libraries->find( $branch ) } ) ) {
         $self->screen_msg('Item cannot be transferred.');
         $self->ok(0);
         return $self;

diff --git a/Koha/Club.pm b/Koha/Club.pm
index 09a510b..f55bfb5 100644 (file)
--- a/Koha/Club.pm
+++ b/Koha/Club.pm
@@ -48,7 +48,7 @@ sub club_template {
 
     return unless $self->club_template_id();
 
-    return scalar Koha::Club::Templates->find( $self->club_template_id() );
+    return Koha::Club::Templates->find( $self->club_template_id() );
 }
 
 =head3 club_fields
@@ -84,7 +84,7 @@ sub branch {
 
     return unless $self->branchcode();
 
-    return scalar Koha::Libraries->find( $self->branchcode() );
+    return Koha::Libraries->find( $self->branchcode() );
 }
 
 =head3 type

diff --git a/Koha/Club/Enrollment.pm b/Koha/Club/Enrollment.pm
index 5b42a0a..5b6be0a 100644 (file)
--- a/Koha/Club/Enrollment.pm
+++ b/Koha/Club/Enrollment.pm
@@ -61,7 +61,7 @@ sub cancel {
 
 sub club {
     my ( $self ) = @_;
-    return scalar Koha::Clubs->find( $self->club_id() );
+    return Koha::Clubs->find( $self->club_id() );
 }
 
 =head3 patron
@@ -70,7 +70,7 @@ sub club {
 
 sub patron {
     my ( $self ) = @_;
-    return scalar Koha::Patrons->find( $self->borrowernumber() );
+    return Koha::Patrons->find( $self->borrowernumber() );
 }
 
 =head3 is_canceled

diff --git a/Koha/Club/Field.pm b/Koha/Club/Field.pm
index 56f5aa0..6f92d83 100644 (file)
--- a/Koha/Club/Field.pm
+++ b/Koha/Club/Field.pm
@@ -46,7 +46,7 @@ Represents the value set at creation time for a Koha::Club::Template::Field
 sub club_template_field {
     my ( $self ) = @_;
 
-    return scalar Koha::Club::Template::Fields->find( $self->club_template_field_id );
+    return Koha::Club::Template::Fields->find( $self->club_template_field_id );
 }
 
 =head3 type

diff --git a/Koha/Objects.pm b/Koha/Objects.pm
index 4b08c79..6d94933 100644 (file)
--- a/Koha/Objects.pm
+++ b/Koha/Objects.pm
@@ -76,6 +76,8 @@ Similar to DBIx::Class::ResultSet->find this method accepts:
 Strictly speaking, columns_values should only refer to columns under an
 unique constraint.
 
+It returns undef if no results were found
+
 my $object = Koha::Objects->find( { col1 => $val1, col2 => $val2 } );
 my $object = Koha::Objects->find( $id );
 my $object = Koha::Objects->find( $idpart1, $idpart2, $attrs ); # composite PK
@@ -85,15 +87,14 @@ my $object = Koha::Objects->find( $idpart1, $idpart2, $attrs ); # composite PK
 sub find {
     my ( $self, @pars ) = @_;
 
-    croak 'Cannot use "->find" in list context' if wantarray;
-
-    return if !@pars || none { defined($_) } @pars;
+    my $object;
 
-    my $result = $self->_resultset()->find( @pars );
-
-    return unless $result;
-
-    my $object = $self->object_class()->_new_from_dbic( $result );
+    unless (!@pars || none { defined($_) } @pars) {
+        my $result = $self->_resultset()->find(@pars);
+        if ($result) {
+            $object = $self->object_class()->_new_from_dbic($result);
+        }
+    }
 
     return $object;
 }

diff --git a/Koha/Subscription.pm b/Koha/Subscription.pm
index 833f674..8213818 100644 (file)
--- a/Koha/Subscription.pm
+++ b/Koha/Subscription.pm
@@ -51,7 +51,7 @@ Returns the biblio linked to this subscription as a Koha::Biblio object
 sub biblio {
     my ($self) = @_;
 
-    return scalar Koha::Biblios->find($self->biblionumber);
+    return Koha::Biblios->find($self->biblionumber);
 }
 
 =head3 vendor
@@ -62,7 +62,7 @@ Returns the vendor/supplier linked to this subscription as a Koha::Acquisition::
 
 sub vendor {
     my ($self) = @_;
-    return scalar Koha::Acquisition::Booksellers->find($self->aqbooksellerid);
+    return Koha::Acquisition::Booksellers->find($self->aqbooksellerid);
 }
 
 =head3 subscribers

diff --git a/acqui/booksellers.pl b/acqui/booksellers.pl
index 99da78c..0b69feb 100755 (executable)
--- a/acqui/booksellers.pl
+++ b/acqui/booksellers.pl
@@ -82,7 +82,7 @@ my $allbaskets= $query->param('allbaskets')||0;
 my @suppliers;
 
 if ($booksellerid) {
-    push @suppliers, scalar Koha::Acquisition::Booksellers->find( $booksellerid );
+    push @suppliers, Koha::Acquisition::Booksellers->find( $booksellerid );
 } else {
     @suppliers = Koha::Acquisition::Booksellers->search(
                         { name => { -like => "%$supplier%" } },

diff --git a/acqui/uncertainprice.pl b/acqui/uncertainprice.pl
index 4a559e3..5ecf41d 100755 (executable)
--- a/acqui/uncertainprice.pl
+++ b/acqui/uncertainprice.pl
@@ -72,7 +72,7 @@ my $op = $input->param('op');
 my $owner = $input->param('owner') || 0 ; # flag to see only "my" orders, or everyone orders
 my $bookseller = Koha::Acquisition::Booksellers->find( $booksellerid );
 
-$template->param( basket => scalar Koha::Acquisition::Baskets->find($basketno) );
+$template->param( basket => Koha::Acquisition::Baskets->find($basketno) );
 
 #show all orders that have uncertain price for the bookseller
 my $pendingorders = SearchOrders({

diff --git a/cataloguing/moveitem.pl b/cataloguing/moveitem.pl
index 1e664a9..18474ac 100755 (executable)
--- a/cataloguing/moveitem.pl
+++ b/cataloguing/moveitem.pl
@@ -79,7 +79,7 @@ if ( $barcode && $biblionumber ) {
         if ($moveresult) {
             $template->param(
                 success => 1,
-                from_biblio => scalar Koha::Biblios->find($frombiblionumber),
+                from_biblio => Koha::Biblios->find($frombiblionumber),
             );
         }
         else {

diff --git a/circ/overdue.pl b/circ/overdue.pl
index f7ef53e..a61a795 100755 (executable)
--- a/circ/overdue.pl
+++ b/circ/overdue.pl
@@ -309,7 +309,7 @@ if ($noreport) {
         }
 
         push @overduedata, {
-            patron                 => scalar Koha::Patrons->find( $data->{borrowernumber} ),
+            patron                 => Koha::Patrons->find( $data->{borrowernumber} ),
             duedate                => $data->{date_due},
             borrowernumber         => $data->{borrowernumber},
             cardnumber             => $data->{cardnumber},

diff --git a/members/memberentry.pl b/members/memberentry.pl
index c2c05fb..902beeb 100755 (executable)
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -804,7 +804,7 @@ $template->param( csrf_token =>
 
 # HouseboundModule data
 $template->param(
-    housebound_role  => scalar Koha::Patron::HouseboundRoles->find($borrowernumber),
+    housebound_role  => Koha::Patron::HouseboundRoles->find($borrowernumber),
 );
 
 if(defined($data{'flags'})){

diff --git a/members/pay.pl b/members/pay.pl
index a126bfe..e70e195 100755 (executable)
--- a/members/pay.pl
+++ b/members/pay.pl
@@ -115,7 +115,7 @@ elsif ( $input->param('confirm_writeoff') ) {
         $payment_id = Koha::Account->new( { patron_id => $borrowernumber } )->pay(
             {
                 amount     => $amount,
-                lines      => [ scalar Koha::Account::Lines->find($accountlines_id) ],
+                lines      => [ Koha::Account::Lines->find($accountlines_id) ],
                 type       => 'WRITEOFF',
                 note       => $payment_note,
                 interface  => C4::Context->interface,
@@ -222,7 +222,7 @@ sub writeoff_all {
             Koha::Account->new( { patron_id => $borrowernumber } )->pay(
                 {
                     amount => $amount,
-                    lines  => [ scalar Koha::Account::Lines->find($accountlines_id) ],
+                    lines  => [ Koha::Account::Lines->find($accountlines_id) ],
                     type   => 'WRITEOFF',
                     note   => $payment_note,
                     interface  => C4::Context->interface,

diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t
index d40d26b..27dd449 100644 (file)
--- a/t/db_dependent/Koha/Objects.t
+++ b/t/db_dependent/Koha/Objects.t
@@ -46,13 +46,20 @@ my $borrowernumber_exists = grep { /^borrowernumber$/ } @columns;
 is( $borrowernumber_exists, 1, 'Koha::Objects->columns should return the table columns' );
 
 subtest 'find' => sub {
-    plan tests => 4;
+    plan tests => 6;
     my $patron = $builder->build({source => 'Borrower'});
     my $patron_object = Koha::Patrons->find( $patron->{borrowernumber} );
     is( $patron_object->borrowernumber, $patron->{borrowernumber}, '->find should return the correct object' );
 
-    eval { my @patrons = Koha::Patrons->find( $patron->{borrowernumber} ); };
-    like( $@, qr|^Cannot use "->find" in list context|, "->find should not be called in list context to avoid side-effects" );
+    my @patrons = Koha::Patrons->find( $patron->{borrowernumber} );
+    is(scalar @patrons, 1, '->find in list context returns a value');
+    is($patrons[0]->borrowernumber, $patron->{borrowernumber}, '->find in list context returns the same value as in scalar context');
+
+    my $patrons = {
+        foo => Koha::Patrons->find('foo'),
+        bar => 'baz',
+    };
+    is ($patrons->{foo}, undef, '->find in list context returns undef when no record is found');
 
     # Test sending undef to find; should not generate a warning
     warning_is { $patron = Koha::Patrons->find( undef ); }