Bugzilla – Attachment 91287 Details for
Bug 19809
Koha::Objects::find no longer need to be forbidden in list context
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[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), 11.99 KB, created by
Julian Maurice
on 2019-07-04 11:12:54 UTC
(
hide
)
Description:
Bug 19809: Re-allow to call Koha::Objects::find in list context
Filename:
MIME Type:
Creator:
Julian Maurice
Created:
2019-07-04 11:12:54 UTC
Size:
11.99 KB
patch
obsolete
>From 54787dee7cb480c600af07ff5058d822263b716c 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. >Also, fix Koha::Patron::guarantor which had the same problem as find > >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/Patron.pm | 7 +++++-- > 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 ++-- > opac/opac-memberentry.pl | 2 +- > t/db_dependent/Koha/Objects.t | 13 ++++++++++--- > 16 files changed, 41 insertions(+), 31 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 097a5c09d6..93f1022c63 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -800,7 +800,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/Patron.pm b/Koha/Patron.pm >index b1d9b14620..dd560ebf95 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -374,9 +374,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 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 697057ef03..97ac16ee4b 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 0b6f0f802a..e112c6dd9f 100755 >--- a/circ/overdue.pl >+++ b/circ/overdue.pl >@@ -299,7 +299,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 bd86e36b67..f944925e06 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -800,7 +800,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 dd7a1efb5f..df4cb91d0e 100755 >--- a/members/pay.pl >+++ b/members/pay.pl >@@ -110,7 +110,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, >@@ -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, > interface => C4::Context->interface, >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index f76a30c098..06dc8da572 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -312,7 +312,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, 'edit' ), > csrf_token => Koha::Token->new->generate_csrf({ > session_id => scalar $cgi->cookie('CGISESSID'), >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 19809
:
69790
|
72977
|
91287
|
92825
|
95115
|
97700
|
97701