Bug 26993: last_returned_by_all should return an array ref

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Lucas Gass 2025-09-23 20:35:13 +00:00
parent c0f1b747c4
commit e25ecf6757
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 20 additions and 18 deletions

View File

@ -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

View File

@ -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' );
};