From 1fd77268034129b777a87130826bdfe73bb8064b Mon Sep 17 00:00:00 2001 From: Lucas Gass Date: Tue, 23 Sep 2025 20:35:13 +0000 Subject: [PATCH] Bug 26993: last_returned_by_all should return an array ref Signed-off-by: Trevor Diamond --- Koha/Item.pm | 4 ++- .../Circulation/StoreLastBorrower.t | 34 +++++++++---------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index a85aa60abd0..37dec7fc552 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -946,7 +946,9 @@ sub last_returned_by_all { } ); - return map { Koha::Patron->_new_from_dbic( $_->borrowernumber ) } @borrowers; + my @stored_borrowers = map { Koha::Patron->_new_from_dbic( $_->borrowernumber ) } @borrowers; + + return \@stored_borrowers; } =head3 last_returned_by diff --git a/t/db_dependent/Circulation/StoreLastBorrower.t b/t/db_dependent/Circulation/StoreLastBorrower.t index f4a5a9f7821..48648716e2b 100755 --- a/t/db_dependent/Circulation/StoreLastBorrower.t +++ b/t/db_dependent/Circulation/StoreLastBorrower.t @@ -152,8 +152,8 @@ subtest 'Test StoreLastBorrower with multiple borrowers' => sub { # Test last_returned_by_all with no borrowers t::lib::Mocks::mock_preference( 'StoreLastBorrower', '3' ); - my @borrowers = $item->last_returned_by_all(); - is( scalar(@borrowers), 0, 'last_returned_by_all returns empty array when no borrowers stored' ); + my $borrowers = $item->last_returned_by_all(); + is( scalar(@$borrowers), 0, 'last_returned_by_all returns empty array when no borrowers stored' ); # Add 3 borrowers for testing, checkout/check in my @patrons; @@ -183,13 +183,13 @@ subtest 'Test StoreLastBorrower with multiple borrowers' => sub { $item = $item->get_from_storage; # Test last_returned_by_all returns all borrowers - @borrowers = $item->last_returned_by_all(); - is( scalar(@borrowers), 3, 'Correctly returns 3 borrowers' ); + $borrowers = $item->last_returned_by_all(); + is( scalar(@$borrowers), 3, 'Correctly returns 3 borrowers' ); # Test ordering - is( $borrowers[0]->borrowernumber, $patrons[2]->{borrowernumber}, 'Most recent borrower first' ); - is( $borrowers[1]->borrowernumber, $patrons[1]->{borrowernumber}, 'Second most recent borrower second' ); - is( $borrowers[2]->borrowernumber, $patrons[0]->{borrowernumber}, 'Oldest borrower last' ); + is( $borrowers->[0]->borrowernumber, $patrons[2]->{borrowernumber}, 'Most recent borrower first' ); + is( $borrowers->[1]->borrowernumber, $patrons[1]->{borrowernumber}, 'Second most recent borrower second' ); + is( $borrowers->[2]->borrowernumber, $patrons[0]->{borrowernumber}, 'Oldest borrower last' ); # Add 2 more borrowers/check out/check in for my $i ( 4 .. 5 ) { @@ -216,12 +216,12 @@ subtest 'Test StoreLastBorrower with multiple borrowers' => sub { } $item = $item->get_from_storage; - @borrowers = $item->last_returned_by_all(); + $borrowers = $item->last_returned_by_all(); is( - scalar(@borrowers), 3, + scalar(@$borrowers), 3, 'We only retain 3 borrowers when the sys pref is set to 3, even though there are 5 checkouts/checkins' ); - is( $borrowers[0]->borrowernumber, $patrons[4]->{borrowernumber}, 'Most recent borrower after cleanup' ); + is( $borrowers->[0]->borrowernumber, $patrons[4]->{borrowernumber}, 'Most recent borrower after cleanup' ); # Reduce StoreLastBorrower to 2 t::lib::Mocks::mock_preference( 'StoreLastBorrower', '2' ); @@ -249,10 +249,10 @@ subtest 'Test StoreLastBorrower with multiple borrowers' => sub { ); $item = $item->get_from_storage; - @borrowers = $item->last_returned_by_all(); - is( scalar(@borrowers), 2, 'StoreLastBorrower was reduced to 2, we should now only keep 2 in the table' ); + $borrowers = $item->last_returned_by_all(); + is( scalar(@$borrowers), 2, 'StoreLastBorrower was reduced to 2, we should now only keep 2 in the table' ); is( - $borrowers[0]->borrowernumber, $yet_another_patron->{borrowernumber}, + $borrowers->[0]->borrowernumber, $yet_another_patron->{borrowernumber}, 'Most recent borrower after limit reduction' ); @@ -283,8 +283,8 @@ subtest 'Test StoreLastBorrower with multiple borrowers' => sub { ); $item = $item->get_from_storage; - @borrowers = $item->last_returned_by_all(); - is( scalar(@borrowers), 0, 'last_returned_by_all respects preference value 0' ); + $borrowers = $item->last_returned_by_all(); + is( scalar(@$borrowers), 0, 'last_returned_by_all respects preference value 0' ); my $cleanup_patron = $builder->build( { @@ -309,8 +309,8 @@ subtest 'Test StoreLastBorrower with multiple borrowers' => sub { ); $item = $item->get_from_storage; - @borrowers = $item->last_returned_by_all(); - is( scalar(@borrowers), 0, 'All entries cleared when preference is 0' ); + $borrowers = $item->last_returned_by_all(); + is( scalar(@$borrowers), 0, 'All entries cleared when preference is 0' ); is( $item->last_returned_by, undef, 'last_returned_by returns undef when no records' ); }; -- 2.39.5