From 39e61e95c98cd47bfcc61816a84ced2ff258c2bb Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 10 Jan 2022 17:08:24 -0300 Subject: [PATCH] Bug 29844: Fix t/db_dependent/Koha/* Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall --- 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 5f4da3f11ef..f5798d23b84 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 9b402a1b399..af93725d60e 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 8404d4bb42f..ee91925d104 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 cfaeb4583a3..d2d4593479c 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 ddf4ebe5bbb..4fb63afbd88 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 0d922b6fa6a..9bf45aa4ac1 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 56f0fd9dd52..8f021ef3495 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 203183138fe..97845a50fa4 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 e7532209481..b616b4efe18 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 6cb55c29f3a..fbdb4414c68 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 c3b8c1bd6b1..e7b8fed4009 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 a2c0e523208..f57bb0cb367 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 48a258dc7a6..86c68fd5c7d 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 ce114cab0ee..3e829064327 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 3378ccdc0e6..c5f2d6293a8 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 57374616080..5daaa30dae2 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.30.2