Attachment 92825 Details for Bug 19809 – Bug 19809: Re-allow to call Koha::Objects::find in list context

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

Bug-19809-Re-allow-to-call-KohaObjectsfind-in-list.patch (text/plain), 10.71 KB, created by Julian Maurice on 2019-09-16 06:08:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Julian Maurice

Created: 2019-09-16 06:08:38 UTC

Size: 10.71 KB

patch

obsolete

>From 5f49c9b22de670295fc776d50f5276ace1cf5ca6 Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>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 <brendan@bywatersolutions.com>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 19809: 69790 | 72977 | 91287 | 92825 | 95115 | 97700 | 97701