Bugzilla – Attachment 130135 Details for
Bug 29844
Remove uses of wantarray in Koha::Objects
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29844: Fix t/db_dependent/Koha/*
Bug-29844-Fix-tdbdependentKoha.patch (text/plain), 20.08 KB, created by
Martin Renvoize (ashimema)
on 2022-02-03 12:43:55 UTC
(
hide
)
Description:
Bug 29844: Fix t/db_dependent/Koha/*
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2022-02-03 12:43:55 UTC
Size:
20.08 KB
patch
obsolete
>From ca4f80e222f26633aa355320bdb123c68f4d790c Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Mon, 10 Jan 2022 17:08:24 -0300 >Subject: [PATCH] Bug 29844: Fix t/db_dependent/Koha/* > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > Koha/Account.pm | 6 ++--- > Koha/Account/Line.pm | 2 +- > Koha/Acquisition/Currency.pm | 2 +- > Koha/Holds.pm | 4 ++-- > Koha/Patron.pm | 24 +++++++------------ > Koha/Patron/Attributes.pm | 2 +- > Koha/Patron/Relationships.pm | 8 ++----- > Koha/SearchEngine/Elasticsearch.pm | 2 +- > Koha/SearchEngine/Elasticsearch/Search.pm | 6 ++--- > t/db_dependent/Koha/Account.t | 23 ++---------------- > t/db_dependent/Koha/Acquisition/Booksellers.t | 5 ++-- > t/db_dependent/Koha/Biblio.t | 5 +--- > t/db_dependent/Koha/Libraries.t | 4 ++-- > t/db_dependent/Koha/Patron.t | 4 ++-- > t/db_dependent/Koha/Patron/Modifications.t | 2 +- > t/db_dependent/Koha/Patrons.t | 18 ++++++-------- > 16 files changed, 39 insertions(+), 78 deletions(-) > >diff --git a/Koha/Account.pm b/Koha/Account.pm >index 5f4da3f11e..f5798d23b8 100644 >--- a/Koha/Account.pm >+++ b/Koha/Account.pm >@@ -647,8 +647,7 @@ my $lines = Koha::Account->new({ patron_id => $patron_id })->outstanding_debits; > > It returns the debit lines with outstanding amounts for the patron. > >-In scalar context, it returns a Koha::Account::Lines iterator. In list context, it will >-return a list of Koha::Account::Line objects. >+It returns a Koha::Account::Lines iterator. > > =cut > >@@ -669,8 +668,7 @@ my $lines = Koha::Account->new({ patron_id => $patron_id })->outstanding_credits > > It returns the credit lines with outstanding amounts for the patron. > >-In scalar context, it returns a Koha::Account::Lines iterator. In list context, it will >-return a list of Koha::Account::Line objects. >+It returns a Koha::Account::Lines iterator. > > =cut > >diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm >index 9b402a1b39..af93725d60 100644 >--- a/Koha/Account/Line.pm >+++ b/Koha/Account/Line.pm >@@ -285,7 +285,7 @@ sub void { > # Find any applied offsets for the credit so we may reverse them > my @account_offsets = > Koha::Account::Offsets->search( >- { credit_id => $self->id, amount => { '<' => 0 } } ); >+ { credit_id => $self->id, amount => { '<' => 0 } } )->as_list; > > my $void; > $self->_result->result_source->schema->txn_do( >diff --git a/Koha/Acquisition/Currency.pm b/Koha/Acquisition/Currency.pm >index 8404d4bb42..ee91925d10 100644 >--- a/Koha/Acquisition/Currency.pm >+++ b/Koha/Acquisition/Currency.pm >@@ -47,7 +47,7 @@ sub store { > currency => { '!=' => $self->currency }, > active => 1, > } >- ); >+ )->as_list; > for my $currency ( @currencies ) { > $currency->active(0); > $currency->store; >diff --git a/Koha/Holds.pm b/Koha/Holds.pm >index cfaeb4583a..d2d4593479 100644 >--- a/Koha/Holds.pm >+++ b/Koha/Holds.pm >@@ -124,7 +124,7 @@ sub get_items_that_can_fill { > columns => ['itemnumber'], > collapse => 1, > } >- ); >+ )->as_list; > my @waiting_holds = map { $_->itemnumber } > Koha::Holds->search( > { 'found' => 'W' }, >@@ -132,7 +132,7 @@ sub get_items_that_can_fill { > columns => ['itemnumber'], > collapse => 1, > } >- ); >+ )->as_list; > > return Koha::Items->search( > { >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index ddf4ebe5bb..4fb63afbd8 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -518,10 +518,10 @@ sub relationships_debt { > Koha::Exceptions::BadParameter->throw( { parameter => 'only_this_guarantor' } ) unless @guarantors; > } elsif ( $self->guarantor_relationships->count ) { > # I am a guarantee, just get all my guarantors >- @guarantors = $self->guarantor_relationships->guarantors; >+ @guarantors = $self->guarantor_relationships->guarantors->as_list; > } else { > # I am a guarantor, I need to get all the guarantors of all my guarantees >- @guarantors = map { $_->guarantor_relationships->guarantors } $self->guarantee_relationships->guarantees; >+ @guarantors = map { $_->guarantor_relationships->guarantors->as_list } $self->guarantee_relationships->guarantees->as_list; > } > > my $non_issues_charges = 0; >@@ -530,7 +530,7 @@ sub relationships_debt { > $non_issues_charges += $guarantor->account->non_issues_charges if $include_guarantors && !$seen->{ $guarantor->id }; > > # We've added what the guarantor owes, not added in that guarantor's guarantees as well >- my @guarantees = map { $_->guarantee } $guarantor->guarantee_relationships(); >+ my @guarantees = map { $_->guarantee } $guarantor->guarantee_relationships->as_list; > my $guarantees_non_issues_charges = 0; > foreach my $guarantee (@guarantees) { > next if $seen->{ $guarantee->id }; >@@ -583,12 +583,12 @@ Returns the siblings of this patron. > sub siblings { > my ($self) = @_; > >- my @guarantors = $self->guarantor_relationships()->guarantors(); >+ my @guarantors = $self->guarantor_relationships()->guarantors()->as_list; > > return unless @guarantors; > > my @siblings = >- map { $_->guarantee_relationships()->guarantees() } @guarantors; >+ map { $_->guarantee_relationships()->guarantees()->as_list } @guarantors; > > return unless @siblings; > >@@ -596,7 +596,7 @@ sub siblings { > @siblings = > grep { !$seen{ $_->id }++ && ( $_->id != $self->id ) } @siblings; > >- return wantarray ? @siblings : Koha::Patrons->search( { borrowernumber => { -in => [ map { $_->id } @siblings ] } } ); >+ return Koha::Patrons->search( { borrowernumber => { -in => [ map { $_->id } @siblings ] } } ); > } > > =head3 merge_with >@@ -1357,11 +1357,7 @@ sub first_valid_email_address { > sub get_club_enrollments { > my ( $self, $return_scalar ) = @_; > >- my $e = Koha::Club::Enrollments->search( { borrowernumber => $self->borrowernumber(), date_canceled => undef } ); >- >- return $e if $return_scalar; >- >- return wantarray ? $e->as_list : $e; >+ return Koha::Club::Enrollments->search( { borrowernumber => $self->borrowernumber(), date_canceled => undef } ); > } > > =head3 get_enrollable_clubs >@@ -1378,11 +1374,7 @@ sub get_enrollable_clubs { > > $params->{borrower} = $self; > >- my $e = Koha::Clubs->get_enrollable($params); >- >- return $e if $return_scalar; >- >- return wantarray ? $e->as_list : $e; >+ return Koha::Clubs->get_enrollable($params); > } > > =head3 account_locked >diff --git a/Koha/Patron/Attributes.pm b/Koha/Patron/Attributes.pm >index 0d922b6fa6..9bf45aa4ac 100644 >--- a/Koha/Patron/Attributes.pm >+++ b/Koha/Patron/Attributes.pm >@@ -97,7 +97,7 @@ sub merge_and_replace_with { > my ( $self, $new_attributes ) = @_; > > my @existing_attributes = @{$self->unblessed}; >- my $attribute_types = { map { $_->code => $_->unblessed } Koha::Patron::Attribute::Types->search }; >+ my $attribute_types = { map { $_->code => $_->unblessed } Koha::Patron::Attribute::Types->search->as_list }; > my @new_attributes; > for my $attr ( @$new_attributes ) { > >diff --git a/Koha/Patron/Relationships.pm b/Koha/Patron/Relationships.pm >index 56f0fd9dd5..8f021ef349 100644 >--- a/Koha/Patron/Relationships.pm >+++ b/Koha/Patron/Relationships.pm >@@ -52,9 +52,7 @@ sub guarantors { > @guarantor_ids = grep { defined $_ } @guarantor_ids; > @guarantor_ids = uniq( @guarantor_ids ); > >- my $guarantors = Koha::Patrons->search( { borrowernumber => \@guarantor_ids } ); >- >- return wantarray ? $guarantors->as_list : $guarantors; >+ return Koha::Patrons->search( { borrowernumber => \@guarantor_ids } ); > } > > =head3 guarantees >@@ -71,14 +69,12 @@ sub guarantees { > > my @guarantee_ids = uniq( $rs->get_column('guarantee_id')->all() ); > >- my $guarantees = Koha::Patrons->search( >+ return Koha::Patrons->search( > { borrowernumber => \@guarantee_ids }, > { > order_by => { -asc => [ 'surname', 'firstname' ] } > }, > ); >- >- return wantarray ? $guarantees->as_list : $guarantees; > } > > =head3 type >diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm >index 203183138f..97845a50fa 100644 >--- a/Koha/SearchEngine/Elasticsearch.pm >+++ b/Koha/SearchEngine/Elasticsearch.pm >@@ -547,7 +547,7 @@ sub marc_records_to_documents { > > my %auth_match_headings; > if( $self->index eq 'authorities' ){ >- my @auth_types = Koha::Authority::Types->search(); >+ my @auth_types = Koha::Authority::Types->search->as_list; > %auth_match_headings = map { $_->authtypecode => $_->auth_tag_to_report } @auth_types; > } > >diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm >index e753220948..b616b4efe1 100644 >--- a/Koha/SearchEngine/Elasticsearch/Search.pm >+++ b/Koha/SearchEngine/Elasticsearch/Search.pm >@@ -456,10 +456,10 @@ sub _convert_facets { > > # We also have some special cases, e.g. itypes that need to show the > # value rather than the code. >- my @itypes = Koha::ItemTypes->search; >- my @libraries = Koha::Libraries->search; >+ my @itypes = Koha::ItemTypes->search->as_list; >+ my @libraries = Koha::Libraries->search->as_list; > my $library_names = { map { $_->branchcode => $_->branchname } @libraries }; >- my @locations = Koha::AuthorisedValues->search( { category => 'LOC' } ); >+ my @locations = Koha::AuthorisedValues->search( { category => 'LOC' } )->as_list; > my $opac = C4::Context->interface eq 'opac' ; > my %special = ( > itype => { map { $_->itemtype => $_->description } @itypes }, >diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t >index 6cb55c29f3..fbdb4414c6 100755 >--- a/t/db_dependent/Koha/Account.t >+++ b/t/db_dependent/Koha/Account.t >@@ -56,7 +56,7 @@ subtest 'new' => sub { > > subtest 'outstanding_debits() tests' => sub { > >- plan tests => 22; >+ plan tests => 10; > > $schema->storage->txn_begin; > >@@ -70,19 +70,10 @@ subtest 'outstanding_debits() tests' => sub { > push @generated_lines, $account->add_debit({ amount => 4, interface => 'commandline', type => 'OVERDUE' }); > > my $lines = $account->outstanding_debits(); >- my @lines_arr = $account->outstanding_debits(); > > is( ref($lines), 'Koha::Account::Lines', 'Called in scalar context, outstanding_debits returns a Koha::Account::Lines object' ); > is( $lines->total_outstanding, 10, 'Outstandig debits total is correctly calculated' ); > >- my $i = 0; >- foreach my $line ( @{ $lines->as_list } ) { >- my $fetched_line = Koha::Account::Lines->find( $generated_lines[$i]->id ); >- is_deeply( $line->unblessed, $fetched_line->unblessed, "Fetched line matches the generated one ($i)" ); >- is_deeply( $lines_arr[$i]->unblessed, $fetched_line->unblessed, "Fetched line matches the generated one ($i)" ); >- is( ref($lines_arr[$i]), 'Koha::Account::Line', 'outstanding_debits returns a list of Koha::Account::Line objects in list context' ); >- $i++; >- } > my $patron_2 = $builder->build_object({ class => 'Koha::Patrons' }); > Koha::Account::Line->new( > { >@@ -149,7 +140,7 @@ subtest 'outstanding_debits() tests' => sub { > > subtest 'outstanding_credits() tests' => sub { > >- plan tests => 17; >+ plan tests => 5; > > $schema->storage->txn_begin; > >@@ -163,20 +154,10 @@ subtest 'outstanding_credits() tests' => sub { > push @generated_lines, $account->add_credit({ amount => 4, interface => 'commandline' }); > > my $lines = $account->outstanding_credits(); >- my @lines_arr = $account->outstanding_credits(); > > is( ref($lines), 'Koha::Account::Lines', 'Called in scalar context, outstanding_credits returns a Koha::Account::Lines object' ); > is( $lines->total_outstanding, -10, 'Outstandig credits total is correctly calculated' ); > >- my $i = 0; >- foreach my $line ( @{ $lines->as_list } ) { >- my $fetched_line = Koha::Account::Lines->find( $generated_lines[$i]->id ); >- is_deeply( $line->unblessed, $fetched_line->unblessed, "Fetched line matches the generated one ($i)" ); >- is_deeply( $lines_arr[$i]->unblessed, $fetched_line->unblessed, "Fetched line matches the generated one ($i)" ); >- is( ref($lines_arr[$i]), 'Koha::Account::Line', 'outstanding_debits returns a list of Koha::Account::Line objects in list context' ); >- $i++; >- } >- > my $patron_2 = $builder->build_object({ class => 'Koha::Patrons' }); > $account = $patron_2->account; > $lines = $account->outstanding_credits(); >diff --git a/t/db_dependent/Koha/Acquisition/Booksellers.t b/t/db_dependent/Koha/Acquisition/Booksellers.t >index c3b8c1bd6b..e7b8fed400 100755 >--- a/t/db_dependent/Koha/Acquisition/Booksellers.t >+++ b/t/db_dependent/Koha/Acquisition/Booksellers.t >@@ -129,8 +129,9 @@ subtest '->subscriptions() tests' => sub { > > # Re-fetch vendor > $vendor = Koha::Acquisition::Booksellers->find( $vendor->id ); >- is( $vendor->subscriptions->count, 2, 'Vendor has two subscriptions' ); >- foreach my $subscription ( $vendor->subscriptions ) { >+ my $subscriptions = $vendor->subscriptions; >+ is( $subscriptions->count, 2, 'Vendor has two subscriptions' ); >+ while (my $subscription = $subscriptions->next ) { > is( ref($subscription), 'Koha::Subscription', 'Type is correct' ); > } > >diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t >index a2c0e52320..f57bb0cb36 100755 >--- a/t/db_dependent/Koha/Biblio.t >+++ b/t/db_dependent/Koha/Biblio.t >@@ -126,7 +126,7 @@ subtest 'hidden_in_opac() tests' => sub { > > subtest 'items() tests' => sub { > >- plan tests => 4; >+ plan tests => 3; > > $schema->storage->txn_begin; > >@@ -141,9 +141,6 @@ subtest 'items() tests' => sub { > is( ref($items), 'Koha::Items', 'Returns a Koha::Items resultset' ); > is( $items->count, 2, 'Two items in resultset' ); > >- my @items = $biblio->items->as_list; >- is( scalar @items, 2, 'Same result, but in list context' ); >- > $schema->storage->txn_rollback; > > }; >diff --git a/t/db_dependent/Koha/Libraries.t b/t/db_dependent/Koha/Libraries.t >index 48a258dc7a..86c68fd5c7 100755 >--- a/t/db_dependent/Koha/Libraries.t >+++ b/t/db_dependent/Koha/Libraries.t >@@ -300,7 +300,7 @@ subtest 'get_hold_libraries and validate_hold_sibling' => sub { > my @hold_libraries_1 = ($library1, $library2); > my @hold_libraries_2 = ($library3, $library4, $library5); > >- my @result = $library1->get_hold_libraries(); >+ my @result = $library1->get_hold_libraries()->as_list; > # library1 and library2 are siblings > is(scalar(@result), 2, 'get_hold_libraries returns 2 libraries'); > >@@ -310,7 +310,7 @@ subtest 'get_hold_libraries and validate_hold_sibling' => sub { > ok(exists $map{$hold_library->branchcode}, 'library in hold group'); > } > >- @result = $library3->get_hold_libraries(); >+ @result = $library3->get_hold_libraries()->as_list; > # library3, library4 and library5 are siblings > is(scalar(@result), 3, 'get_hold_libraries returns 3 libraries'); > >diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t >index ce114cab0e..3e82906432 100755 >--- a/t/db_dependent/Koha/Patron.t >+++ b/t/db_dependent/Koha/Patron.t >@@ -241,7 +241,7 @@ subtest 'add_enrolment_fee_if_needed() tests' => sub { > $enrollment_fee = $patron->add_enrolment_fee_if_needed(1); > is( $patron->account->balance * 1, 60, 'Patron charged the enrolment fees' ); > >- my @debits = $account->outstanding_debits; >+ my @debits = $account->outstanding_debits->as_list; > is( scalar @debits, 3, '3 enrolment fees' ); > is( $debits[0]->debit_type_code, 'ACCOUNT', 'Account type set correctly' ); > is( $debits[1]->debit_type_code, 'ACCOUNT', 'Account type set correctly' ); >@@ -279,7 +279,7 @@ subtest 'add_enrolment_fee_if_needed() tests' => sub { > my $account = $patron->account; > is( $patron->account->balance, 0, 'Patron not charged anything' ); > >- my @debits = $account->outstanding_debits; >+ my @debits = $account->outstanding_debits->as_list; > is( scalar @debits, 0, 'no debits' ); > > $schema->storage->txn_rollback; >diff --git a/t/db_dependent/Koha/Patron/Modifications.t b/t/db_dependent/Koha/Patron/Modifications.t >index 3378ccdc0e..c5f2d6293a 100755 >--- a/t/db_dependent/Koha/Patron/Modifications.t >+++ b/t/db_dependent/Koha/Patron/Modifications.t >@@ -223,7 +223,7 @@ subtest 'approve tests' => sub { > my @patron_attributes > = map { $_->unblessed } > Koha::Patron::Attributes->search( >- { borrowernumber => $patron->borrowernumber } ); >+ { borrowernumber => $patron->borrowernumber } )->as_list; > > is( $patron_attributes[0]->{code}, > 'CODE_1', 'Untouched attribute type is preserved (code)' ); >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 5737461608..5daaa30dae 100755 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -99,16 +99,14 @@ subtest 'sms_provider' => sub { > }; > > subtest 'guarantees' => sub { >- plan tests => 13; >+ >+ plan tests => 9; > > t::lib::Mocks::mock_preference( 'borrowerRelationship', 'test|test2' ); > > my $guarantees = $new_patron_1->guarantee_relationships; > is( ref($guarantees), 'Koha::Patron::Relationships', 'Koha::Patron->guarantees should return a Koha::Patrons result set in a scalar context' ); > is( $guarantees->count, 0, 'new_patron_1 should have 0 guarantee relationships' ); >- my @guarantees = $new_patron_1->guarantee_relationships; >- is( ref(\@guarantees), 'ARRAY', 'Koha::Patron->guarantee_relationships should return an array in a list context' ); >- is( scalar(@guarantees), 0, 'new_patron_1 should have 0 guarantee' ); > > my $guarantee_1 = $builder->build({ source => 'Borrower' }); > my $relationship_1 = Koha::Patron::Relationship->new( { guarantor_id => $new_patron_1->id, guarantee_id => $guarantee_1->{borrowernumber}, relationship => 'test' } )->store(); >@@ -118,10 +116,8 @@ subtest 'guarantees' => sub { > $guarantees = $new_patron_1->guarantee_relationships; > is( ref($guarantees), 'Koha::Patron::Relationships', 'Koha::Patron->guarantee_relationships should return a Koha::Patrons result set in a scalar context' ); > is( $guarantees->count, 2, 'new_patron_1 should have 2 guarantees' ); >- @guarantees = $new_patron_1->guarantee_relationships; >- is( ref(\@guarantees), 'ARRAY', 'Koha::Patron->guarantee_relationships should return an array in a list context' ); >- is( scalar(@guarantees), 2, 'new_patron_1 should have 2 guarantees' ); >- $_->delete for @guarantees; >+ >+ $guarantees->delete; > > #Test return order of guarantees BZ 18635 > my $categorycode = $builder->build({ source => 'Category' })->{categorycode}; >@@ -246,7 +242,9 @@ subtest 'category' => sub { > }; > > subtest 'siblings' => sub { >- plan tests => 7; >+ >+ plan tests => 6; >+ > my $siblings = $new_patron_1->siblings; > is( $siblings, undef, 'Koha::Patron->siblings should not crashed if the patron has no guarantor' ); > my $guarantee_1 = $builder->build( { source => 'Borrower' } ); >@@ -254,8 +252,6 @@ subtest 'siblings' => sub { > my $retrieved_guarantee_1 = Koha::Patrons->find($guarantee_1); > $siblings = $retrieved_guarantee_1->siblings; > is( ref($siblings), 'Koha::Patrons', 'Koha::Patron->siblings should return a Koha::Patrons result set in a scalar context' ); >- my @siblings = $retrieved_guarantee_1->siblings; >- is( ref( \@siblings ), 'ARRAY', 'Koha::Patron->siblings should return an array in a list context' ); > is( $siblings->count, 0, 'guarantee_1 should not have siblings yet' ); > my $guarantee_2 = $builder->build( { source => 'Borrower' } ); > my $relationship_2 = Koha::Patron::Relationship->new( { guarantor_id => $new_patron_1->borrowernumber, guarantee_id => $guarantee_2->{borrowernumber}, relationship => 'test' } )->store(); >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=130135">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 29844
:
129285
|
129286
|
129287
|
129290
|
129291
|
129292
|
129293
|
129295
|
129996
|
129997
|
129998
|
129999
|
130098
|
130099
|
130100
|
130101
|
130129
|
130130
|
130131
|
130132
|
130133
|
130134
| 130135 |
130136
|
130137
|
130138
|
130139
|
130143
|
130145
|
130146
|
130481
|
130947
|
131032