From 5f49c9b22de670295fc776d50f5276ace1cf5ca6 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. Signed-off-by: Brendan Gallagher --- C4/Reserves.pm | 2 +- C4/SIP/ILS/Transaction/Hold.pm | 2 +- Koha/Club.pm | 4 ++-- Koha/Club/Enrollment.pm | 4 ++-- Koha/Club/Field.pm | 2 +- Koha/Objects.pm | 18 +++++++++--------- Koha/Subscription.pm | 4 ++-- acqui/booksellers.pl | 2 +- acqui/uncertainprice.pl | 2 +- cataloguing/moveitem.pl | 2 +- circ/overdue.pl | 2 +- members/memberentry.pl | 2 +- members/pay.pl | 4 ++-- t/db_dependent/Koha/Objects.t | 13 ++++++++++--- 14 files changed, 35 insertions(+), 28 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 2a77c3dc5b..eba45fd4d1 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -797,7 +797,7 @@ sub CheckReserves { next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $patron->branchcode)); my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy}; next if ( ($branchitemrule->{hold_fulfillment_policy} ne 'any') && ($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 63e1a06dbb..cf27e8098e 100644 --- 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 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 da43003c88..a3251dfd5d 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/Subscription.pm b/Koha/Subscription.pm index 833f674ad4..8213818fa5 100644 --- 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 99da78cb9f..0b69feb497 100755 --- 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 4a559e3691..5ecf41d83c 100755 --- 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 1e664a911e..18474ac343 100755 --- 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 0f286bc563..bc5942b93c 100755 --- a/circ/overdue.pl +++ b/circ/overdue.pl @@ -308,7 +308,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 fa5cbe427e..64f99645c4 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -791,7 +791,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 00e6f7b386..1a67c5a724 100755 --- a/members/pay.pl +++ b/members/pay.pl @@ -112,7 +112,7 @@ elsif ( $input->param('confirm_writeoff') ) { 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, @@ -217,7 +217,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 db7711e0db..51d20649f7 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.20.1