From 99b0cbd10ab2e53813e6999fe2472335fc73a854 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Thu, 14 Dec 2017 09:19:22 +0100 Subject: [PATCH] Bug 19809: Re-allow to call Koha::Objects::find in list context and remove 'scalar' keyword in calls where it's not needed. Also, fix Koha::Patron::guarantor which had the same problem as find --- Koha/Club.pm | 4 ++-- Koha/Club/Enrollment.pm | 4 ++-- Koha/Club/Field.pm | 2 +- Koha/Objects.pm | 18 +++++++++--------- Koha/Patron.pm | 7 +++++-- Koha/Subscription.pm | 2 +- acqui/booksellers.pl | 2 +- acqui/uncertainprice.pl | 2 +- members/memberentry.pl | 2 +- members/pay.pl | 4 ++-- opac/opac-memberentry.pl | 2 +- opac/opac-user.pl | 2 +- t/db_dependent/Koha/Objects.t | 13 ++++++++++--- 13 files changed, 37 insertions(+), 27 deletions(-) diff --git a/Koha/Club.pm b/Koha/Club.pm index 09a510b64d..f55bfb54a8 100644 --- 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 168f55bca6..788f956984 100644 --- a/Koha/Club/Enrollment.pm +++ b/Koha/Club/Enrollment.pm @@ -59,7 +59,7 @@ sub cancel { sub club { my ( $self ) = @_; - return scalar Koha::Clubs->find( $self->club_id() ); + return Koha::Clubs->find( $self->club_id() ); } =head3 patron @@ -68,7 +68,7 @@ sub club { sub patron { my ( $self ) = @_; - return scalar Koha::Patrons->find( $self->borrowernumber() ); + return Koha::Patrons->find( $self->borrowernumber() ); } =head3 type diff --git a/Koha/Club/Field.pm b/Koha/Club/Field.pm index 56f5aa03e8..6f92d83f31 100644 --- 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 8c79afda1a..8aa0db6ab7 100644 --- a/Koha/Objects.pm +++ b/Koha/Objects.pm @@ -85,15 +85,15 @@ 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 $result = $self->_resultset()->find( @pars ); - - return unless $result; - - my $object = $self->object_class()->_new_from_dbic( $result ); + my $object; + + @pars = grep { defined } @pars; + if (@pars) { + my $result = $self->_resultset()->find(@pars); + if ($result) { + $object = $self->object_class()->_new_from_dbic($result); + } + } return $object; } diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 92e70a5b91..6116d8323a 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -117,9 +117,12 @@ Returns a Koha::Patron object for this patron's guarantor sub guarantor { my ( $self ) = @_; - return unless $self->guarantorid(); + my $guarantor; + if ($self->guarantorid()) { + $guarantor = Koha::Patrons->find( $self->guarantorid() ); + } - return Koha::Patrons->find( $self->guarantorid() ); + return $guarantor; } sub image { diff --git a/Koha/Subscription.pm b/Koha/Subscription.pm index 2a793e5946..0398763787 100644 --- a/Koha/Subscription.pm +++ b/Koha/Subscription.pm @@ -45,7 +45,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 type diff --git a/acqui/booksellers.pl b/acqui/booksellers.pl index 8a35b04bd0..16a8163ebb 100755 --- a/acqui/booksellers.pl +++ b/acqui/booksellers.pl @@ -83,7 +83,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 7fa95ab417..220805eb88 100755 --- a/acqui/uncertainprice.pl +++ b/acqui/uncertainprice.pl @@ -73,7 +73,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/members/memberentry.pl b/members/memberentry.pl index 13793e71cf..24d4eb18bb 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -780,7 +780,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 ce35b0a12d..afe03009dc 100755 --- a/members/pay.pl +++ b/members/pay.pl @@ -103,7 +103,7 @@ if ($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, library_id => $branch, @@ -214,7 +214,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, library_id => $branch, diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 33988c113f..18387e81bb 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -305,7 +305,7 @@ elsif ( $action eq 'edit' ) { #Display logged in borrower's data $template->param( borrower => $borrower, - guarantor => scalar Koha::Patrons->find($borrowernumber)->guarantor(), + guarantor => Koha::Patrons->find($borrowernumber)->guarantor(), hidden => GetHiddenFields( $mandatory, 'modification' ), csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $cgi->cookie('CGISESSID'), diff --git a/opac/opac-user.pl b/opac/opac-user.pl index 55c04a3a78..5e68e649f8 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -345,7 +345,7 @@ if ( C4::Context->preference('AllowPatronToSetCheckoutsVisibilityForGuarantor' } $template->param( - borrower => scalar Koha::Patrons->find($borrowernumber), + borrower => Koha::Patrons->find($borrowernumber), patron_messages => $patron_messages, opacnote => $borr->{opacnote}, patronupdate => $patronupdate, diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t index 9c41d9f145..4e3cdd3c56 100644 --- 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 ); } -- 2.11.0